Sulkta-OSS/element-x-ada
3
0
Fork 0
Code Issues 1 Pull requests 10 Projects Releases Packages Wiki Activity Actions

Confirm exit without saving change in room details edit screen (#5618)

Browse source 
* 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 <android@element.io>
This commit is contained in:
Benoit Marty 2025-10-28 18:25:51 +01:00 • committed by GitHub
parent 4d9d684f2a
commit 08f75dda4c
20 changed files with 270 additions and 153 deletions
Download patch file Download diff file Expand all files Collapse all files

4
features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesEvent.kt
View file

@ -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
}

42
features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenter.kt
View file

@ -69,20 +69,19 @@ class ChangeRolesPresenter(
val selectedUsers = remember {
mutableStateOf<ImmutableList<MatrixUser>>(persistentListOf())
}
val exitState: MutableState<AsyncAction<Unit>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val saveState: MutableState<AsyncAction<Unit>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val saveState: MutableState<AsyncAction<Boolean>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val usersWithRole = produceState<ImmutableList<MatrixUser>>(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<MatrixUser>,
selectedUsers: MutableState<ImmutableList<MatrixUser>>,
saveState: MutableState<AsyncAction<Unit>>,
saveState: MutableState<AsyncAction<Boolean>>,
) = 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() }
}

11
features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesState.kt
View file

@ -23,8 +23,7 @@ data class ChangeRolesState(
val searchResults: SearchBarResultState<MembersByRole>,
val selectedUsers: ImmutableList<MatrixUser>,
val hasPendingChanges: Boolean,
val exitState: AsyncAction<Unit>,
val savingState: AsyncAction<Unit>,
val savingState: AsyncAction<Boolean>,
val canChangeMemberRole: (UserId) -> Boolean,
val eventSink: (ChangeRolesEvent) -> Unit,
)
@ -36,10 +35,10 @@ data class MembersByRole(
val members: ImmutableList<RoomMember>,
) {
constructor(members: List<RoomMember>) : 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()

8
features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesStateProvider.kt
View file

@ -38,10 +38,10 @@ class ChangeRolesStateProvider : PreviewParameterProvider<ChangeRolesState> {
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<MembersByRole> = SearchBarResultState.NoResultsFound(),
selectedUsers: ImmutableList<MatrixUser> = persistentListOf(),
hasPendingChanges: Boolean = false,
exitState: AsyncAction<Unit> = AsyncAction.Uninitialized,
savingState: AsyncAction<Unit> = AsyncAction.Uninitialized,
savingState: AsyncAction<Boolean> = 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,

103
features/changeroommemberroles/impl/src/main/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesView.kt
View file

@ -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)
},
)
}
}

37
features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesPresenterTest.kt
View file

@ -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)
}
}

10
features/changeroommemberroles/impl/src/test/kotlin/io/element/android/features/changeroommemberroles/impl/ChangeRolesViewTest.kt
View file

@ -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