Address review comments.

This commit is contained in:
Benoit Marty 2025-09-26 16:28:13 +02:00
parent 3214607ce5
commit e26aae4ac8
4 changed files with 30 additions and 32 deletions

View file

@ -27,7 +27,10 @@ import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.spaces.SpaceRoom import io.element.android.libraries.matrix.api.spaces.SpaceRoom
import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.ImmutableSet
import kotlinx.collections.immutable.persistentSetOf
import kotlinx.collections.immutable.toPersistentList import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentSet
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlin.jvm.optionals.getOrNull import kotlin.jvm.optionals.getOrNull
@ -52,13 +55,13 @@ class LeaveSpacePresenter(
mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized) mutableStateOf<AsyncAction<Unit>>(AsyncAction.Uninitialized)
} }
val selectedRoomIds = remember { val selectedRoomIds = remember {
mutableStateOf<Set<RoomId>>(emptySet()) mutableStateOf<ImmutableSet<RoomId>>(persistentSetOf())
} }
val joinedSpaceRooms by produceState(emptyList()) { val joinedSpaceRooms by produceState(emptyList()) {
// TODO Get the joined room from the SDK, should also have the // TODO Get the joined room from the SDK, should also have the isLastAdmin boolean
val rooms = emptyList<SpaceRoom>() val rooms = emptyList<SpaceRoom>()
// By default select all rooms // By default select all rooms
selectedRoomIds.value = rooms.map { it.roomId }.toSet() selectedRoomIds.value = rooms.map { it.roomId }.toPersistentSet()
value = rooms value = rooms
} }
val selectableSpaceRooms by produceState<AsyncData<ImmutableList<SelectableSpaceRoom>>>( val selectableSpaceRooms by produceState<AsyncData<ImmutableList<SelectableSpaceRoom>>>(
@ -81,14 +84,14 @@ class LeaveSpacePresenter(
fun handleEvents(event: LeaveSpaceEvents) { fun handleEvents(event: LeaveSpaceEvents) {
when (event) { when (event) {
LeaveSpaceEvents.DeselectAllRooms -> { LeaveSpaceEvents.DeselectAllRooms -> {
selectedRoomIds.value = emptySet() selectedRoomIds.value = persistentSetOf()
} }
LeaveSpaceEvents.SelectAllRooms -> { LeaveSpaceEvents.SelectAllRooms -> {
selectedRoomIds.value = selectableSpaceRooms.dataOrNull() selectedRoomIds.value = selectableSpaceRooms.dataOrNull()
.orEmpty() .orEmpty()
.filter { it.isLastAdmin.not() } .filter { it.isLastAdmin.not() }
.map { it.spaceRoom.roomId } .map { it.spaceRoom.roomId }
.toSet() .toPersistentSet()
} }
is LeaveSpaceEvents.ToggleRoomSelection -> { is LeaveSpaceEvents.ToggleRoomSelection -> {
val currentSet = selectedRoomIds.value val currentSet = selectedRoomIds.value
@ -97,6 +100,7 @@ class LeaveSpacePresenter(
} else { } else {
currentSet + event.roomId currentSet + event.roomId
} }
.toPersistentSet()
} }
LeaveSpaceEvents.LeaveSpace -> coroutineScope.leaveSpace( LeaveSpaceEvents.LeaveSpace -> coroutineScope.leaveSpace(
leaveSpaceAction = leaveSpaceAction, leaveSpaceAction = leaveSpaceAction,

View file

@ -18,33 +18,27 @@ data class LeaveSpaceState(
val eventSink: (LeaveSpaceEvents) -> Unit, val eventSink: (LeaveSpaceEvents) -> Unit,
) { ) {
private val rooms = selectableSpaceRooms.dataOrNull().orEmpty() private val rooms = selectableSpaceRooms.dataOrNull().orEmpty()
private val partition = rooms.partition { it.isLastAdmin }
private val lastAdminRooms = partition.first
private val selectableRooms = partition.second
/** /**
* True if we should show the quick action to select/deselect all rooms. * True if we should show the quick action to select/deselect all rooms.
*/ */
val showQuickAction = rooms val showQuickAction = selectableRooms.isNotEmpty()
.any { !it.isLastAdmin }
/** /**
* True if there are rooms and they are all selected. * True if there all the selectable rooms are selected.
*/ */
val areAllSelected = rooms val areAllSelected = selectableRooms.all { it.isSelected }
.filter { !it.isLastAdmin }
.let { rooms ->
rooms.isNotEmpty() && rooms.all { it.isSelected }
}
/** /**
* True if there are rooms but the user is the last admin in all of them. * True if there are rooms but the user is the last admin in all of them.
*/ */
val hasOnlyLastAdminRoom = rooms val hasOnlyLastAdminRoom = lastAdminRooms.isNotEmpty() && selectableRooms.isEmpty()
.let { rooms ->
rooms.isNotEmpty() && rooms.all { it.isLastAdmin }
}
/** /**
* Number of selected rooms. * Number of selected rooms.
*/ */
val numberOfSelectRooms = rooms val selectedRoomsCount = selectableRooms.count { it.isSelected }
.count { it.isSelected }
} }

View file

@ -118,7 +118,7 @@ fun LeaveSpaceView(
} }
LeaveSpaceButtons( LeaveSpaceButtons(
showLeaveButton = state.selectableSpaceRooms is AsyncData.Success, showLeaveButton = state.selectableSpaceRooms is AsyncData.Success,
nbOfSelectedRooms = state.numberOfSelectRooms, selectedRoomsCount = state.selectedRoomsCount,
onLeaveSpace = { onLeaveSpace = {
state.eventSink(LeaveSpaceEvents.LeaveSpace) state.eventSink(LeaveSpaceEvents.LeaveSpace)
}, },
@ -198,7 +198,7 @@ private fun LeaveSpaceHeader(
@Composable @Composable
private fun LeaveSpaceButtons( private fun LeaveSpaceButtons(
showLeaveButton: Boolean, showLeaveButton: Boolean,
nbOfSelectedRooms: Int, selectedRoomsCount: Int,
onLeaveSpace: () -> Unit, onLeaveSpace: () -> Unit,
onCancel: () -> Unit, onCancel: () -> Unit,
) { ) {
@ -206,8 +206,8 @@ private fun LeaveSpaceButtons(
modifier = Modifier.padding(top = 16.dp) modifier = Modifier.padding(top = 16.dp)
) { ) {
if (showLeaveButton) { if (showLeaveButton) {
val text = if (nbOfSelectedRooms > 0) { val text = if (selectedRoomsCount > 0) {
pluralStringResource(R.plurals.screen_leave_space_submit, nbOfSelectedRooms, nbOfSelectedRooms) pluralStringResource(R.plurals.screen_leave_space_submit, selectedRoomsCount, selectedRoomsCount)
} else { } else {
stringResource(CommonStrings.action_leave_space) stringResource(CommonStrings.action_leave_space)
} }

View file

@ -20,9 +20,9 @@ class LeaveSpaceStateTest {
selectableSpaceRooms = AsyncData.Loading() selectableSpaceRooms = AsyncData.Loading()
) )
assertThat(sut.showQuickAction).isFalse() assertThat(sut.showQuickAction).isFalse()
assertThat(sut.areAllSelected).isFalse() assertThat(sut.areAllSelected).isTrue()
assertThat(sut.hasOnlyLastAdminRoom).isFalse() assertThat(sut.hasOnlyLastAdminRoom).isFalse()
assertThat(sut.numberOfSelectRooms).isEqualTo(0) assertThat(sut.selectedRoomsCount).isEqualTo(0)
} }
@Test @Test
@ -33,9 +33,9 @@ class LeaveSpaceStateTest {
) )
) )
assertThat(sut.showQuickAction).isFalse() assertThat(sut.showQuickAction).isFalse()
assertThat(sut.areAllSelected).isFalse() assertThat(sut.areAllSelected).isTrue()
assertThat(sut.hasOnlyLastAdminRoom).isFalse() assertThat(sut.hasOnlyLastAdminRoom).isFalse()
assertThat(sut.numberOfSelectRooms).isEqualTo(0) assertThat(sut.selectedRoomsCount).isEqualTo(0)
} }
@Test @Test
@ -51,7 +51,7 @@ class LeaveSpaceStateTest {
assertThat(sut.showQuickAction).isTrue() assertThat(sut.showQuickAction).isTrue()
assertThat(sut.areAllSelected).isFalse() assertThat(sut.areAllSelected).isFalse()
assertThat(sut.hasOnlyLastAdminRoom).isFalse() assertThat(sut.hasOnlyLastAdminRoom).isFalse()
assertThat(sut.numberOfSelectRooms).isEqualTo(1) assertThat(sut.selectedRoomsCount).isEqualTo(1)
} }
@Test @Test
@ -67,7 +67,7 @@ class LeaveSpaceStateTest {
assertThat(sut.showQuickAction).isTrue() assertThat(sut.showQuickAction).isTrue()
assertThat(sut.areAllSelected).isTrue() assertThat(sut.areAllSelected).isTrue()
assertThat(sut.hasOnlyLastAdminRoom).isFalse() assertThat(sut.hasOnlyLastAdminRoom).isFalse()
assertThat(sut.numberOfSelectRooms).isEqualTo(2) assertThat(sut.selectedRoomsCount).isEqualTo(2)
} }
@Test @Test
@ -84,7 +84,7 @@ class LeaveSpaceStateTest {
assertThat(sut.showQuickAction).isTrue() assertThat(sut.showQuickAction).isTrue()
assertThat(sut.areAllSelected).isTrue() assertThat(sut.areAllSelected).isTrue()
assertThat(sut.hasOnlyLastAdminRoom).isFalse() assertThat(sut.hasOnlyLastAdminRoom).isFalse()
assertThat(sut.numberOfSelectRooms).isEqualTo(2) assertThat(sut.selectedRoomsCount).isEqualTo(2)
} }
@Test @Test
@ -98,8 +98,8 @@ class LeaveSpaceStateTest {
) )
) )
assertThat(sut.showQuickAction).isFalse() assertThat(sut.showQuickAction).isFalse()
assertThat(sut.areAllSelected).isFalse() assertThat(sut.areAllSelected).isTrue()
assertThat(sut.hasOnlyLastAdminRoom).isTrue() assertThat(sut.hasOnlyLastAdminRoom).isTrue()
assertThat(sut.numberOfSelectRooms).isEqualTo(0) assertThat(sut.selectedRoomsCount).isEqualTo(0)
} }
} }