From 08f75dda4c8cdd8a34a740093aa7695dbbfbe1a6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 28 Oct 2025 18:25:51 +0100 Subject: [PATCH] Confirm exit without saving change in room details edit screen (#5618) * Room details edit screen: add confirmation dialog when leaving without saving pending changes. * Improve preview coverage. * Update screenshots * Introduce AsyncAction.ConfirmingCancellation and use it for leaving room edition without saving change. * Fix issue in comment * Use new `ConfirmingCancellation` object in Change Roles screen --------- Co-authored-by: ElementBot --- .../impl/ChangeRolesEvent.kt | 4 +- .../impl/ChangeRolesPresenter.kt | 42 +++---- .../impl/ChangeRolesState.kt | 11 +- .../impl/ChangeRolesStateProvider.kt | 8 +- .../impl/ChangeRolesView.kt | 103 +++++++++--------- .../impl/ChangeRolesPresenterTest.kt | 37 +++---- .../impl/ChangeRolesViewTest.kt | 10 +- .../impl/edit/RoomDetailsEditEvents.kt | 3 +- .../impl/edit/RoomDetailsEditNode.kt | 3 +- .../impl/edit/RoomDetailsEditPresenter.kt | 8 +- .../impl/edit/RoomDetailsEditStateProvider.kt | 1 + .../impl/edit/RoomDetailsEditView.kt | 36 ++++-- .../members/RoomMemberListStateProvider.kt | 3 +- .../impl/edit/RoomDetailsEditPresenterTest.kt | 79 +++++++++++++- .../impl/edit/RoomDetailsEditViewTest.kt | 56 +++++++--- .../libraries/architecture/AsyncAction.kt | 5 + ...impl.edit_RoomDetailsEditView_Day_8_en.png | 3 + ...pl.edit_RoomDetailsEditView_Night_8_en.png | 3 + ...pl.members_RoomMemberListView_Day_1_en.png | 4 +- ....members_RoomMemberListView_Night_1_en.png | 4 +- 20 files changed, 270 insertions(+), 153 deletions(-) create mode 100644 tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Day_8_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Night_8_en.png diff --git a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesEvent.kt b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesEvent.kt index ab8dbc8f22..56e1c50bcc 100644 --- a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesEvent.kt +++ b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesEvent.kt @@ -15,7 +15,5 @@ sealed interface ChangeRolesEvent { data class UserSelectionToggled(val matrixUser: MatrixUser) : ChangeRolesEvent data object Save : ChangeRolesEvent data object Exit : ChangeRolesEvent - data object CancelExit : ChangeRolesEvent - data object ClearError : ChangeRolesEvent - data object CancelSave : ChangeRolesEvent + data object CloseDialog : ChangeRolesEvent } diff --git a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenter.kt b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenter.kt index 632d93f0e0..b6d865a66a 100644 --- a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenter.kt +++ b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenter.kt @@ -69,20 +69,19 @@ class ChangeRolesPresenter( val selectedUsers = remember { mutableStateOf>(persistentListOf()) } - val exitState: MutableState> = remember { mutableStateOf(AsyncAction.Uninitialized) } - val saveState: MutableState> = remember { mutableStateOf(AsyncAction.Uninitialized) } + val saveState: MutableState> = remember { mutableStateOf(AsyncAction.Uninitialized) } val usersWithRole = produceState>(initialValue = persistentListOf()) { room.usersWithRole(role).map { members -> members.map { it.toMatrixUser() } } - .onEach { users -> - val previous = value - value = users.toImmutableList() - // Users who were selected but didn't have the role, so their role change was pending - val toAdd = selectedUsers.value.filter { user -> users.none { it.userId == user.userId } && previous.none { it.userId == user.userId } } - // Users who no longer have the role - val toRemove = previous.filter { user -> users.none { it.userId == user.userId } }.toSet() - selectedUsers.value = (users + toAdd - toRemove).toImmutableList() - } - .launchIn(this) + .onEach { users -> + val previous = value + value = users.toImmutableList() + // Users who were selected but didn't have the role, so their role change was pending + val toAdd = selectedUsers.value.filter { user -> users.none { it.userId == user.userId } && previous.none { it.userId == user.userId } } + // Users who no longer have the role + val toRemove = previous.filter { user -> users.none { it.userId == user.userId } }.toSet() + selectedUsers.value = (users + toAdd - toRemove).toImmutableList() + } + .launchIn(this) } val roomMemberState by room.membersStateFlow.collectAsState() @@ -147,22 +146,16 @@ class ChangeRolesPresenter( } } } - is ChangeRolesEvent.ClearError -> { - saveState.value = AsyncAction.Uninitialized - } is ChangeRolesEvent.Exit -> { - exitState.value = if (exitState.value.isUninitialized() && hasPendingChanges) { + saveState.value = if (saveState.value.isUninitialized() && hasPendingChanges) { // Has pending changes, confirm exit - AsyncAction.ConfirmingNoParams + AsyncAction.ConfirmingCancellation } else { // No pending changes, exit immediately - AsyncAction.Success(Unit) + AsyncAction.Success(false) } } - is ChangeRolesEvent.CancelExit -> { - exitState.value = AsyncAction.Uninitialized - } - is ChangeRolesEvent.CancelSave -> { + is ChangeRolesEvent.CloseDialog -> { saveState.value = AsyncAction.Uninitialized } } @@ -174,7 +167,6 @@ class ChangeRolesPresenter( searchResults = searchResults, selectedUsers = selectedUsers.value, hasPendingChanges = hasPendingChanges, - exitState = exitState.value, savingState = saveState.value, canChangeMemberRole = ::canChangeMemberRole, eventSink = ::handleEvent, @@ -198,7 +190,7 @@ class ChangeRolesPresenter( private fun CoroutineScope.save( usersWithRole: ImmutableList, selectedUsers: MutableState>, - saveState: MutableState>, + saveState: MutableState>, ) = launch { saveState.value = AsyncAction.Loading @@ -221,7 +213,7 @@ class ChangeRolesPresenter( saveState.value = AsyncAction.Failure(it) } .onSuccess { - saveState.value = AsyncAction.Success(Unit) + saveState.value = AsyncAction.Success(true) // Asynchronously reload the room members launch { room.updateMembers() } } diff --git a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesState.kt b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesState.kt index 027ef76e69..e0b3e68a9e 100644 --- a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesState.kt +++ b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesState.kt @@ -23,8 +23,7 @@ data class ChangeRolesState( val searchResults: SearchBarResultState, val selectedUsers: ImmutableList, val hasPendingChanges: Boolean, - val exitState: AsyncAction, - val savingState: AsyncAction, + val savingState: AsyncAction, val canChangeMemberRole: (UserId) -> Boolean, val eventSink: (ChangeRolesEvent) -> Unit, ) @@ -36,10 +35,10 @@ data class MembersByRole( val members: ImmutableList, ) { constructor(members: List) : this( - owners = members.filter { it.role is RoomMember.Role.Owner }.sorted(), - admins = members.filter { it.role == RoomMember.Role.Admin }.sorted(), - moderators = members.filter { it.role == RoomMember.Role.Moderator }.sorted(), - members = members.filter { it.role == RoomMember.Role.User }.sorted(), + owners = members.filter { it.role is RoomMember.Role.Owner }.sorted(), + admins = members.filter { it.role == RoomMember.Role.Admin }.sorted(), + moderators = members.filter { it.role == RoomMember.Role.Moderator }.sorted(), + members = members.filter { it.role == RoomMember.Role.User }.sorted(), ) fun isEmpty() = owners.isEmpty() && admins.isEmpty() && moderators.isEmpty() && members.isEmpty() diff --git a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesStateProvider.kt b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesStateProvider.kt index 2041c0f447..b54347f73f 100644 --- a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesStateProvider.kt +++ b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesStateProvider.kt @@ -38,10 +38,10 @@ class ChangeRolesStateProvider : PreviewParameterProvider { searchResults = SearchBarResultState.Results(MembersByRole(aRoomMemberList().take(1).toImmutableList())), selectedUsers = aMatrixUserList().take(1).toImmutableList(), ), - aChangeRolesStateWithSelectedUsers().copy(exitState = AsyncAction.ConfirmingNoParams), + aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.ConfirmingCancellation), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.ConfirmingNoParams), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Loading), - aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Success(Unit)), + aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Success(true)), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Failure(Exception("boom"))), aChangeRolesStateWithOwners(), aChangeRolesStateWithOwners().copy(role = RoomMember.Role.Owner(isCreator = false)), @@ -55,8 +55,7 @@ internal fun aChangeRolesState( searchResults: SearchBarResultState = SearchBarResultState.NoResultsFound(), selectedUsers: ImmutableList = persistentListOf(), hasPendingChanges: Boolean = false, - exitState: AsyncAction = AsyncAction.Uninitialized, - savingState: AsyncAction = AsyncAction.Uninitialized, + savingState: AsyncAction = AsyncAction.Uninitialized, canRemoveMember: (UserId) -> Boolean = { true }, eventSink: (ChangeRolesEvent) -> Unit = {}, ) = ChangeRolesState( @@ -66,7 +65,6 @@ internal fun aChangeRolesState( searchResults = searchResults, selectedUsers = selectedUsers, hasPendingChanges = hasPendingChanges, - exitState = exitState, savingState = savingState, canChangeMemberRole = canRemoveMember, eventSink = eventSink, diff --git a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesView.kt b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesView.kt index c6b70a82f4..f9ebd75ca2 100644 --- a/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesView.kt +++ b/features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesView.kt @@ -29,7 +29,6 @@ import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Alignment @@ -41,7 +40,6 @@ import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import io.element.android.compound.theme.ElementTheme import io.element.android.libraries.architecture.AsyncAction -import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.async.AsyncActionView import io.element.android.libraries.designsystem.components.async.AsyncIndicator import io.element.android.libraries.designsystem.components.async.AsyncIndicatorHost @@ -52,7 +50,6 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.components.avatar.AvatarType import io.element.android.libraries.designsystem.components.button.BackButton import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog -import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.theme.components.Checkbox @@ -172,61 +169,59 @@ fun ChangeRolesView( AsyncIndicatorHost(modifier = Modifier.statusBarsPadding(), asyncIndicatorState) AsyncActionView( - async = state.exitState, - onSuccess = { latestNavigateUp() }, - confirmationDialog = { - ConfirmationDialog( - title = stringResource(CommonStrings.dialog_unsaved_changes_title), - content = stringResource(CommonStrings.dialog_unsaved_changes_description_android), - onSubmitClick = { state.eventSink(ChangeRolesEvent.Exit) }, - onDismiss = { state.eventSink(ChangeRolesEvent.CancelExit) } - ) - }, - onErrorDismiss = { /* Cannot happen */ }, - ) - - when (state.savingState) { - is AsyncAction.Confirming -> { - when (state.role) { - is RoomMember.Role.Owner -> { - ConfirmationDialog( - title = stringResource(R.string.screen_room_change_role_confirm_change_owners_title), - content = stringResource(R.string.screen_room_change_role_confirm_change_owners_description), - submitText = stringResource(CommonStrings.action_continue), - onSubmitClick = { state.eventSink(ChangeRolesEvent.Save) }, - onDismiss = { state.eventSink(ChangeRolesEvent.ClearError) }, - destructiveSubmit = true, - ) - } - is RoomMember.Role.Admin -> { - ConfirmationDialog( - title = stringResource(R.string.screen_room_change_role_confirm_add_admin_title), - content = stringResource(R.string.screen_room_change_role_confirm_add_admin_description), - onSubmitClick = { state.eventSink(ChangeRolesEvent.Save) }, - onDismiss = { state.eventSink(ChangeRolesEvent.ClearError) } - ) - } - else -> Unit // No confirmation needed for Moderator or User roles - } - } - is AsyncAction.Loading -> { - ProgressDialog() - } - is AsyncAction.Failure -> { - ErrorDialog( - content = stringResource(CommonStrings.error_unknown), - onSubmit = { state.eventSink(ChangeRolesEvent.ClearError) } - ) - } - is AsyncAction.Success -> { - LaunchedEffect(state.savingState) { + async = state.savingState, + onSuccess = { changeSaved -> + if (changeSaved) { asyncIndicatorState.enqueue(durationMs = AsyncIndicator.DURATION_SHORT) { AsyncIndicator.Custom(text = stringResource(CommonStrings.common_saved_changes)) } + } else { + latestNavigateUp() } - } - else -> Unit - } + }, + confirmationDialog = { confirming -> + when (confirming) { + is AsyncAction.ConfirmingCancellation -> { + ConfirmationDialog( + title = stringResource(CommonStrings.dialog_unsaved_changes_title), + content = stringResource(CommonStrings.dialog_unsaved_changes_description_android), + onSubmitClick = { state.eventSink(ChangeRolesEvent.Exit) }, + onDismiss = { state.eventSink(ChangeRolesEvent.CloseDialog) } + ) + } + else -> { + when (state.role) { + is RoomMember.Role.Owner -> { + ConfirmationDialog( + title = stringResource(R.string.screen_room_change_role_confirm_change_owners_title), + content = stringResource(R.string.screen_room_change_role_confirm_change_owners_description), + submitText = stringResource(CommonStrings.action_continue), + onSubmitClick = { state.eventSink(ChangeRolesEvent.Save) }, + onDismiss = { state.eventSink(ChangeRolesEvent.CloseDialog) }, + destructiveSubmit = true, + ) + } + is RoomMember.Role.Admin -> { + ConfirmationDialog( + title = stringResource(R.string.screen_room_change_role_confirm_add_admin_title), + content = stringResource(R.string.screen_room_change_role_confirm_add_admin_description), + onSubmitClick = { state.eventSink(ChangeRolesEvent.Save) }, + onDismiss = { state.eventSink(ChangeRolesEvent.CloseDialog) } + ) + } + // No confirmation needed for Moderator or User roles + else -> Unit + } + } + } + }, + errorMessage = { + stringResource(CommonStrings.error_unknown) + }, + onErrorDismiss = { + state.eventSink(ChangeRolesEvent.CloseDialog) + }, + ) } } diff --git a/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenterTest.kt b/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenterTest.kt index 41b6acd60c..a5e4cfaccb 100644 --- a/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenterTest.kt +++ b/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenterTest.kt @@ -52,7 +52,6 @@ class ChangeRolesPresenterTest { assertThat(searchResults).isInstanceOf(SearchBarResultState.Initial::class.java) assertThat(selectedUsers).isEmpty() assertThat(hasPendingChanges).isFalse() - assertThat(exitState).isEqualTo(AsyncAction.Uninitialized) assertThat(savingState).isEqualTo(AsyncAction.Uninitialized) } cancelAndIgnoreRemainingEvents() @@ -266,7 +265,7 @@ class ChangeRolesPresenterTest { } @Test - fun `present - Exit will display success if no pending changes`() = runTest { + fun `present - Exit will display success false if no pending changes`() = runTest { val room = FakeJoinedRoom().apply { givenRoomMembersState(RoomMembersState.Ready(aRoomMemberList())) givenRoomInfo(aRoomInfo(roomPowerLevels = roomPowerLevelsWithRole(RoomMember.Role.Admin))) @@ -278,15 +277,15 @@ class ChangeRolesPresenterTest { skipItems(1) val initialState = awaitItem() assertThat(initialState.hasPendingChanges).isFalse() - assertThat(initialState.exitState).isEqualTo(AsyncAction.Uninitialized) + assertThat(initialState.savingState).isEqualTo(AsyncAction.Uninitialized) initialState.eventSink(ChangeRolesEvent.Exit) - assertThat(awaitItem().exitState).isEqualTo(AsyncAction.Success(Unit)) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(false)) } } @Test - fun `present - CancelExit will remove exit confirmation`() = runTest { + fun `present - CloseDialog will remove exit confirmation`() = runTest { val room = FakeJoinedRoom().apply { givenRoomMembersState(RoomMembersState.Ready(aRoomMemberList())) givenRoomInfo(aRoomInfo(roomPowerLevels = roomPowerLevelsWithRole(RoomMember.Role.Admin))) @@ -298,16 +297,16 @@ class ChangeRolesPresenterTest { skipItems(1) val initialState = awaitItem() assertThat(initialState.hasPendingChanges).isFalse() - assertThat(initialState.exitState).isEqualTo(AsyncAction.Uninitialized) + assertThat(initialState.savingState).isEqualTo(AsyncAction.Uninitialized) initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) awaitItem().eventSink(ChangeRolesEvent.Exit) val confirmingState = awaitItem() - assertThat(confirmingState.exitState).isEqualTo(AsyncAction.ConfirmingNoParams) + assertThat(confirmingState.savingState).isEqualTo(AsyncAction.ConfirmingCancellation) - confirmingState.eventSink(ChangeRolesEvent.CancelExit) - assertThat(awaitItem().exitState).isEqualTo(AsyncAction.Uninitialized) + confirmingState.eventSink(ChangeRolesEvent.CloseDialog) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Uninitialized) } } @@ -324,7 +323,7 @@ class ChangeRolesPresenterTest { skipItems(1) val initialState = awaitItem() assertThat(initialState.hasPendingChanges).isFalse() - assertThat(initialState.exitState).isEqualTo(AsyncAction.Uninitialized) + assertThat(initialState.savingState).isEqualTo(AsyncAction.Uninitialized) initialState.eventSink(ChangeRolesEvent.UserSelectionToggled(MatrixUser(A_USER_ID_2))) val updatedState = awaitItem() @@ -332,10 +331,10 @@ class ChangeRolesPresenterTest { skipItems(1) updatedState.eventSink(ChangeRolesEvent.Exit) - assertThat(awaitItem().exitState).isEqualTo(AsyncAction.ConfirmingNoParams) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.ConfirmingCancellation) updatedState.eventSink(ChangeRolesEvent.Exit) - assertThat(awaitItem().exitState).isEqualTo(AsyncAction.Success(Unit)) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(false)) } } @@ -367,12 +366,12 @@ class ChangeRolesPresenterTest { assertThat(loadingState.savingState).isInstanceOf(AsyncAction.Loading::class.java) skipItems(1) - assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(Unit)) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(true)) } } @Test - fun `present - CancelSave will remove the confirmation dialog`() = runTest { + fun `present - CloseDialog will remove the confirmation dialog`() = runTest { val room = FakeJoinedRoom().apply { givenRoomMembersState(RoomMembersState.Ready(aRoomMemberList())) givenRoomInfo(aRoomInfo(roomPowerLevels = roomPowerLevelsWithRole(RoomMember.Role.Admin))) @@ -391,7 +390,7 @@ class ChangeRolesPresenterTest { val confirmingState = awaitItem() assertThat(confirmingState.savingState).isEqualTo(AsyncAction.ConfirmingNoParams) - confirmingState.eventSink(ChangeRolesEvent.CancelSave) + confirmingState.eventSink(ChangeRolesEvent.CloseDialog) assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Uninitialized) } } @@ -426,7 +425,7 @@ class ChangeRolesPresenterTest { assertThat(loadingState.savingState).isInstanceOf(AsyncAction.Loading::class.java) skipItems(1) - assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(Unit)) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(true)) assertThat(analyticsService.capturedEvents.last()).isEqualTo(RoomModeration(RoomModeration.Action.ChangeMemberRole, RoomModeration.Role.Moderator)) } } @@ -504,13 +503,13 @@ class ChangeRolesPresenterTest { assertThat(loadingState.savingState).isInstanceOf(AsyncAction.Loading::class.java) skipItems(1) - assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(Unit)) + assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Success(true)) assertThat(analyticsService.capturedEvents.last()).isEqualTo(RoomModeration(RoomModeration.Action.ChangeMemberRole, RoomModeration.Role.User)) } } @Test - fun `present - Save can handle failures and ClearError clears them`() = runTest { + fun `present - Save can handle failures and CloseDialog clears them`() = runTest { val room = FakeJoinedRoom( updateUserRoleResult = { Result.failure(IllegalStateException("Failed")) } ).apply { @@ -534,7 +533,7 @@ class ChangeRolesPresenterTest { val failedState = awaitItem() assertThat(failedState.savingState).isInstanceOf(AsyncAction.Failure::class.java) - failedState.eventSink(ChangeRolesEvent.ClearError) + failedState.eventSink(ChangeRolesEvent.CloseDialog) assertThat(awaitItem().savingState).isEqualTo(AsyncAction.Uninitialized) } } diff --git a/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesViewTest.kt b/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesViewTest.kt index fb1dc38aae..19d9cadfa8 100644 --- a/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesViewTest.kt +++ b/features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesViewTest.kt @@ -135,7 +135,7 @@ class ChangeRolesViewTest { rule.setChangeRolesContent( state = aChangeRolesState( isSearchActive = true, - exitState = AsyncAction.ConfirmingNoParams, + savingState = AsyncAction.ConfirmingCancellation, eventSink = eventsRecorder, ), ) @@ -151,14 +151,14 @@ class ChangeRolesViewTest { rule.setChangeRolesContent( state = aChangeRolesState( isSearchActive = true, - exitState = AsyncAction.ConfirmingNoParams, + savingState = AsyncAction.ConfirmingCancellation, eventSink = eventsRecorder, ), ) rule.clickOn(CommonStrings.action_cancel) - eventsRecorder.assertSingle(ChangeRolesEvent.CancelExit) + eventsRecorder.assertSingle(ChangeRolesEvent.CloseDialog) } @Test @@ -209,7 +209,7 @@ class ChangeRolesViewTest { rule.clickOn(CommonStrings.action_cancel) - eventsRecorder.assertSingle(ChangeRolesEvent.ClearError) + eventsRecorder.assertSingle(ChangeRolesEvent.CloseDialog) } @Test @@ -225,7 +225,7 @@ class ChangeRolesViewTest { rule.clickOn(CommonStrings.action_ok) - eventsRecorder.assertSingle(ChangeRolesEvent.ClearError) + eventsRecorder.assertSingle(ChangeRolesEvent.CloseDialog) } @Test diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditEvents.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditEvents.kt index 1becbe9e5c..a892de1498 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditEvents.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditEvents.kt @@ -13,6 +13,7 @@ sealed interface RoomDetailsEditEvents { data class HandleAvatarAction(val action: AvatarAction) : RoomDetailsEditEvents data class UpdateRoomName(val name: String) : RoomDetailsEditEvents data class UpdateRoomTopic(val topic: String) : RoomDetailsEditEvents + data object OnBackPress : RoomDetailsEditEvents data object Save : RoomDetailsEditEvents - data object CancelSaveChanges : RoomDetailsEditEvents + data object CloseDialog : RoomDetailsEditEvents } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditNode.kt index 8143f1848f..37fcfce53b 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditNode.kt @@ -41,8 +41,7 @@ class RoomDetailsEditNode( val state = presenter.present() RoomDetailsEditView( state = state, - onBackClick = ::navigateUp, - onRoomEditSuccess = ::navigateUp, + onDone = ::navigateUp, modifier = modifier, ) } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt index 6e318d5f45..cc207d8b24 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt @@ -169,7 +169,13 @@ class RoomDetailsEditPresenter( is RoomDetailsEditEvents.UpdateRoomName -> roomRawNameEdited = event.name is RoomDetailsEditEvents.UpdateRoomTopic -> roomTopicEdited = event.topic - RoomDetailsEditEvents.CancelSaveChanges -> saveAction.value = AsyncAction.Uninitialized + RoomDetailsEditEvents.CloseDialog -> saveAction.value = AsyncAction.Uninitialized + RoomDetailsEditEvents.OnBackPress -> if (saveButtonEnabled.not() || saveAction.value == AsyncAction.ConfirmingCancellation) { + // No changes to save or already confirming exit without saving + saveAction.value = AsyncAction.Success(Unit) + } else { + saveAction.value = AsyncAction.ConfirmingCancellation + } } } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt index f1dcc2463d..3f58e40b98 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditStateProvider.kt @@ -26,6 +26,7 @@ open class RoomDetailsEditStateProvider : PreviewParameterProvider Unit, - onRoomEditSuccess: () -> Unit, + onDone: () -> Unit, modifier: Modifier = Modifier, ) { val focusManager = LocalFocusManager.current @@ -62,12 +64,21 @@ fun RoomDetailsEditView( isAvatarActionsSheetVisible.value = true } + BackHandler { + state.eventSink(RoomDetailsEditEvents.OnBackPress) + } Scaffold( modifier = modifier.clearFocusOnTap(focusManager), topBar = { TopAppBar( titleStr = stringResource(id = R.string.screen_room_details_edit_room_title), - navigationIcon = { BackButton(onClick = onBackClick) }, + navigationIcon = { + BackButton( + onClick = { + state.eventSink(RoomDetailsEditEvents.OnBackPress) + } + ) + }, actions = { TextButton( text = stringResource(CommonStrings.action_save), @@ -126,14 +137,12 @@ fun RoomDetailsEditView( ) } } - AvatarActionBottomSheet( actions = state.avatarActions, isVisible = isAvatarActionsSheetVisible.value, onDismiss = { isAvatarActionsSheetVisible.value = false }, onSelectAction = { state.eventSink(RoomDetailsEditEvents.HandleAvatarAction(it)) } ) - AsyncActionView( async = state.saveAction, progressDialog = { @@ -141,9 +150,19 @@ fun RoomDetailsEditView( progressText = stringResource(R.string.screen_room_details_updating_room), ) }, - onSuccess = { onRoomEditSuccess() }, + confirmationDialog = { + if (state.saveAction == AsyncAction.ConfirmingCancellation) { + ConfirmationDialog( + title = stringResource(CommonStrings.dialog_unsaved_changes_title), + content = stringResource(CommonStrings.dialog_unsaved_changes_description_android), + onSubmitClick = { state.eventSink(RoomDetailsEditEvents.OnBackPress) }, + onDismiss = { state.eventSink(RoomDetailsEditEvents.CloseDialog) } + ) + } + }, + onSuccess = { onDone() }, errorMessage = { stringResource(R.string.screen_room_details_edition_error) }, - onErrorDismiss = { state.eventSink(RoomDetailsEditEvents.CancelSaveChanges) } + onErrorDismiss = { state.eventSink(RoomDetailsEditEvents.CloseDialog) } ) PermissionsView( @@ -156,7 +175,6 @@ fun RoomDetailsEditView( internal fun RoomDetailsEditViewPreview(@PreviewParameter(RoomDetailsEditStateProvider::class) state: RoomDetailsEditState) = ElementPreview { RoomDetailsEditView( state = state, - onBackClick = {}, - onRoomEditSuccess = {}, + onDone = {}, ) } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt index a1cc9c3b92..74e1b8548a 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt @@ -41,7 +41,8 @@ internal class RoomMemberListStateProvider : PreviewParameterProvider Result.success(true) } + ) + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { + val initialState = awaitFirstItem() + assertThat(initialState.saveButtonEnabled).isFalse() + // Once a change is made, the save button is enabled + initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("Name edited")) + awaitItem().apply { + assertThat(saveButtonEnabled).isTrue() + eventSink(RoomDetailsEditEvents.OnBackPress) + } + awaitItem().apply { + assertThat(saveAction).isEqualTo(AsyncAction.ConfirmingCancellation) + eventSink(RoomDetailsEditEvents.CloseDialog) + } + awaitItem().apply { + assertThat(saveAction).isEqualTo(AsyncAction.Uninitialized) + } + } + } + + @Test + fun `present - leave no changes, no confirmation`() = runTest { + val room = aJoinedRoom( + displayName = "Name", + canSendStateResult = { _, _ -> Result.success(true) } + ) + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter {}, + ) + presenter.test { + val initialState = awaitFirstItem() + assertThat(initialState.saveButtonEnabled).isFalse() + initialState.eventSink(RoomDetailsEditEvents.OnBackPress) + assertThat(awaitItem().saveAction).isEqualTo(AsyncAction.Success(Unit)) + } + } + + @Test + fun `present - leave without saving - confirm`() = runTest { + val room = aJoinedRoom( + displayName = "Name", + canSendStateResult = { _, _ -> Result.success(true) } + ) + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter({}), + ) + presenter.test { + val initialState = awaitFirstItem() + assertThat(initialState.saveButtonEnabled).isFalse() + // Once a change is made, the save button is enabled + initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("Name edited")) + awaitItem().apply { + assertThat(saveButtonEnabled).isTrue() + eventSink(RoomDetailsEditEvents.OnBackPress) + } + awaitItem().apply { + assertThat(saveAction).isEqualTo(AsyncAction.ConfirmingCancellation) + eventSink(RoomDetailsEditEvents.OnBackPress) + } + awaitItem().apply { + assertThat(saveAction).isEqualTo(AsyncAction.Success(Unit)) + } + } + } + private suspend fun saveAndAssertFailure( room: JoinedRoom, event: RoomDetailsEditEvents, diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditViewTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditViewTest.kt index 2ac3d4397c..bfd3b5e7cc 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditViewTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditViewTest.kt @@ -38,17 +38,41 @@ class RoomDetailsEditViewTest { @get:Rule val rule = createAndroidComposeRule() @Test - fun `clicking on back invoke back callback`() { - val eventsRecorder = EventsRecorder(expectEvents = false) - ensureCalledOnce { callback -> - rule.setRoomDetailsEditView( - aRoomDetailsEditState( - eventSink = eventsRecorder - ), - onBackClick = callback, - ) - rule.pressBack() - } + fun `clicking on back emits the expected Event`() { + val eventsRecorder = EventsRecorder() + rule.setRoomDetailsEditView( + aRoomDetailsEditState( + eventSink = eventsRecorder + ), + ) + rule.pressBack() + eventsRecorder.assertSingle(RoomDetailsEditEvents.OnBackPress) + } + + @Test + fun `clicking on OK when confirming exit emits the expected Event`() { + val eventsRecorder = EventsRecorder() + rule.setRoomDetailsEditView( + aRoomDetailsEditState( + saveAction = AsyncAction.ConfirmingCancellation, + eventSink = eventsRecorder, + ), + ) + rule.clickOn(CommonStrings.action_ok) + eventsRecorder.assertSingle(RoomDetailsEditEvents.OnBackPress) + } + + @Test + fun `clicking on cancel when confirming exit emits the expected Event`() { + val eventsRecorder = EventsRecorder() + rule.setRoomDetailsEditView( + aRoomDetailsEditState( + saveAction = AsyncAction.ConfirmingCancellation, + eventSink = eventsRecorder, + ), + ) + rule.clickOn(CommonStrings.action_cancel) + eventsRecorder.assertSingle(RoomDetailsEditEvents.CloseDialog) } @Test @@ -60,7 +84,7 @@ class RoomDetailsEditViewTest { eventSink = eventsRecorder, saveAction = AsyncAction.Success(Unit) ), - onRoomEdited = callback, + onDone = callback, ) } } @@ -209,20 +233,18 @@ class RoomDetailsEditViewTest { ), ) rule.clickOn(CommonStrings.action_ok) - eventsRecorder.assertSingle(RoomDetailsEditEvents.CancelSaveChanges) + eventsRecorder.assertSingle(RoomDetailsEditEvents.CloseDialog) } } private fun AndroidComposeTestRule.setRoomDetailsEditView( state: RoomDetailsEditState, - onBackClick: () -> Unit = EnsureNeverCalled(), - onRoomEdited: () -> Unit = EnsureNeverCalled(), + onDone: () -> Unit = EnsureNeverCalled(), ) { setContent { RoomDetailsEditView( state = state, - onBackClick = onBackClick, - onRoomEditSuccess = onRoomEdited, + onDone = onDone, ) } } diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt index c02c64135d..88fa4079f5 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/AsyncAction.kt @@ -32,6 +32,11 @@ sealed interface AsyncAction { data object ConfirmingNoParams : Confirming + /** + * User cancels the action, use this object to ask for confirmation. + */ + data object ConfirmingCancellation : Confirming + /** * Represents an operation that is currently ongoing. */ diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Day_8_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Day_8_en.png new file mode 100644 index 0000000000..3064144e10 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Day_8_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2cd78ece31258d6ee07eabc282f935e6228893d52d24e00e0bbd129138ed8c31 +size 30836 diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Night_8_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Night_8_en.png new file mode 100644 index 0000000000..7888efbfa8 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.edit_RoomDetailsEditView_Night_8_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2bf7bc223f5d481ec8bd1f2a6711751a3bb82ac8f0d856cb5f6c8f4ee7ef50e1 +size 28891 diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_1_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_1_en.png index 459b225859..842147e3f0 100644 --- a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Day_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:37940d14ca14d65256ea220cfd7f0685ba2d100425f65af4d4cffe35be4b69fb -size 45485 +oid sha256:0cd12c68415f61b198696a9f9d8b19da5e6ced287e015071a8a850100238862b +size 49992 diff --git a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_1_en.png b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_1_en.png index b063a6d741..ad5b94c86d 100644 --- a/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_1_en.png +++ b/tests/uitests/src/test/snapshots/images/features.roomdetails.impl.members_RoomMemberListView_Night_1_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4c45c582fb6698e70b376f832b9564946a2271a6b3532886239dfd75d8bb9755 -size 45409 +oid sha256:be897269967951ea0ddb0cc209005d07d4904531a8ab9d865b6e622c9ba98e18 +size 49747