From 82c3d5a689e870adc92ffe00f844d82914f5ba8e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 17:31:17 +0100 Subject: [PATCH 1/4] Render Owner in the horizontal list when editing Admins. Closes #5634. --- .../impl/roles/ChangeRolesStateProvider.kt | 2 -- .../impl/roles/ChangeRolesView.kt | 19 ++++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt index 28c74012d6..07fdd64c68 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt @@ -126,8 +126,6 @@ internal fun aChangeRolesStateWithOwners() = aChangeRolesState( } }, selectedUsers = persistentListOf( - aMatrixUser(id = "@alice:server.org", displayName = "Alice"), - aMatrixUser(id = "@bob:server.org", displayName = "Bob"), aMatrixUser(id = "@carol:server.org", displayName = "Carol"), ) ) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt index 81107e2e89..2eff60fe6e 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt @@ -30,6 +30,9 @@ 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.derivedStateOf +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource @@ -71,6 +74,7 @@ import io.element.android.libraries.matrix.ui.components.SelectedUsersRowList import io.element.android.libraries.matrix.ui.model.getAvatarData import io.element.android.libraries.ui.strings.CommonStrings import kotlinx.collections.immutable.ImmutableList +import kotlinx.collections.immutable.toImmutableList @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -226,7 +230,20 @@ private fun SearchResultsList( state = lazyListState, ) { item { - selectedUsersList(selectedUsers) + if (currentRole == RoomMember.Role.Admin) { + val ownersAndSelectedUsers by remember { + derivedStateOf { + // Also include the owners in the horizontal list + val owners = searchResults.owners.map { + it.toMatrixUser() + } + (owners + selectedUsers).toImmutableList() + } + } + selectedUsersList(ownersAndSelectedUsers) + } else { + selectedUsersList(selectedUsers) + } } if (searchResults.owners.isNotEmpty()) { stickyHeader { ListSectionHeader(text = stringResource(R.string.screen_room_roles_and_permissions_owners)) } From a6201f2008e32916059a239e44dd2d7c16c7484c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 17 Nov 2025 09:20:24 +0100 Subject: [PATCH 2/4] Fix detekt issue Content slots should not be reused in different code branches/scopes of a composable function, to preserve the slot internal state. --- .../impl/roles/ChangeRolesView.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt index 2eff60fe6e..bade3b72c7 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt @@ -230,20 +230,20 @@ private fun SearchResultsList( state = lazyListState, ) { item { - if (currentRole == RoomMember.Role.Admin) { - val ownersAndSelectedUsers by remember { - derivedStateOf { + val usersInHorizontalRow by remember { + derivedStateOf { + if (currentRole == RoomMember.Role.Admin) { // Also include the owners in the horizontal list val owners = searchResults.owners.map { it.toMatrixUser() } (owners + selectedUsers).toImmutableList() + } else { + selectedUsers } } - selectedUsersList(ownersAndSelectedUsers) - } else { - selectedUsersList(selectedUsers) } + selectedUsersList(usersInHorizontalRow) } if (searchResults.owners.isNotEmpty()) { stickyHeader { ListSectionHeader(text = stringResource(R.string.screen_room_roles_and_permissions_owners)) } From 65ff88117404d5f31f549d68369c9bc6c76e6e57 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 17 Nov 2025 09:59:47 +0100 Subject: [PATCH 3/4] Fix owners not displayed in the horizontal list. --- .../impl/roles/ChangeRolesView.kt | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt index bade3b72c7..81b11aa837 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesView.kt @@ -30,8 +30,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.derivedStateOf -import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -230,17 +228,15 @@ private fun SearchResultsList( state = lazyListState, ) { item { - val usersInHorizontalRow by remember { - derivedStateOf { - if (currentRole == RoomMember.Role.Admin) { - // Also include the owners in the horizontal list - val owners = searchResults.owners.map { - it.toMatrixUser() - } - (owners + selectedUsers).toImmutableList() - } else { - selectedUsers + val usersInHorizontalRow = remember(searchResults.owners, selectedUsers) { + if (currentRole == RoomMember.Role.Admin) { + // Also include the owners in the horizontal list + val owners = searchResults.owners.map { + it.toMatrixUser() } + (owners + selectedUsers).toImmutableList() + } else { + selectedUsers } } selectedUsersList(usersInHorizontalRow) From 6e66d63f53569878b092c9bd8ffa35f686f3c74d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 17 Nov 2025 10:46:38 +0100 Subject: [PATCH 4/4] Fix regression on Preview. --- .../impl/roles/ChangeRolesStateProvider.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt index 07fdd64c68..14662bacdd 100644 --- a/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt +++ b/features/rolesandpermissions/impl/src/main/kotlin/io/element/android/features/rolesandpermissions/impl/roles/ChangeRolesStateProvider.kt @@ -44,8 +44,14 @@ class ChangeRolesStateProvider : PreviewParameterProvider { aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Loading), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Success(true)), aChangeRolesStateWithSelectedUsers().copy(savingState = AsyncAction.Failure(Exception("boom"))), - aChangeRolesStateWithOwners(), - aChangeRolesStateWithOwners().copy(role = RoomMember.Role.Owner(isCreator = false)), + aChangeRolesStateWithOwners( + role = RoomMember.Role.Admin, + // Do not include the owners in the selectedUsers (the presenter will not do it), the View will add them + selectedUsers = listOf( + aMatrixUser(id = "@carol:server.org", displayName = "Carol"), + ) + ), + aChangeRolesStateWithOwners(role = RoomMember.Role.Owner(isCreator = false)), ) } @@ -88,8 +94,15 @@ internal fun aChangeRolesStateWithSelectedUsers() = aChangeRolesState( canRemoveMember = { it != UserId("@alice:server.org") }, ) -internal fun aChangeRolesStateWithOwners() = aChangeRolesState( - role = RoomMember.Role.Admin, +internal fun aChangeRolesStateWithOwners( + role: RoomMember.Role = RoomMember.Role.Admin, + selectedUsers: List = listOf( + aMatrixUser(id = "@alice:server.org", displayName = "Alice"), + aMatrixUser(id = "@bob:server.org", displayName = "Bob"), + aMatrixUser(id = "@carol:server.org", displayName = "Carol"), + ), +) = aChangeRolesState( + role = role, searchResults = SearchBarResultState.Results( MembersByRole( members = persistentListOf( @@ -125,7 +138,5 @@ internal fun aChangeRolesStateWithOwners() = aChangeRolesState( else -> false } }, - selectedUsers = persistentListOf( - aMatrixUser(id = "@carol:server.org", displayName = "Carol"), - ) + selectedUsers = selectedUsers.toImmutableList(), )