mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-03 16:10:00 +00:00
Render an error in the room select screen if the rooms cannot be loaded
If the client of the selected account cannot be restored, the room list was never loaded and there was no feedback to the user. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+9
-6
@@ -87,20 +87,22 @@ class RoomSelectPresenter(
|
|||||||
val coroutineScope = rememberCoroutineScope()
|
val coroutineScope = rememberCoroutineScope()
|
||||||
// Create one data source per session. When the selected session changes, the previous
|
// Create one data source per session. When the selected session changes, the previous
|
||||||
// data source is removed from the composition and its coroutine scope is cancelled.
|
// data source is removed from the composition and its coroutine scope is cancelled.
|
||||||
val dataSource = key(selectedSessionId) {
|
val (dataSource, hasRoomListError) = key(selectedSessionId) {
|
||||||
val dataSourceCoroutineScope = rememberCoroutineScope()
|
val dataSourceCoroutineScope = rememberCoroutineScope()
|
||||||
// The sessions are restored at startup, so the client should already be in memory
|
// The sessions are restored at startup, so the client should already be in memory.
|
||||||
val roomListService by produceState(matrixClientProvider.getOrNull(selectedSessionId)?.roomListService) {
|
// A null value means that the client is being restored.
|
||||||
|
val roomListServiceResult by produceState(matrixClientProvider.getOrNull(selectedSessionId)?.roomListService?.let { Result.success(it) }) {
|
||||||
if (value == null) {
|
if (value == null) {
|
||||||
value = matrixClientProvider.getOrRestore(selectedSessionId)
|
value = matrixClientProvider.getOrRestore(selectedSessionId)
|
||||||
.onFailure { Timber.e(it, "Failed to get the client for $selectedSessionId") }
|
.onFailure { Timber.e(it, "Failed to get the client for $selectedSessionId") }
|
||||||
.getOrNull()
|
.map { it.roomListService }
|
||||||
?.roomListService
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
remember(roomListService) {
|
val roomListService = roomListServiceResult?.getOrNull()
|
||||||
|
val dataSource = remember(roomListService) {
|
||||||
roomListService?.let { dataSourceFactory.create(dataSourceCoroutineScope, it) }
|
roomListService?.let { dataSourceFactory.create(dataSourceCoroutineScope, it) }
|
||||||
}
|
}
|
||||||
|
dataSource to (roomListServiceResult?.isFailure == true)
|
||||||
}
|
}
|
||||||
|
|
||||||
val searchQuery = queryState.text.toString()
|
val searchQuery = queryState.text.toString()
|
||||||
@@ -156,6 +158,7 @@ class RoomSelectPresenter(
|
|||||||
isSearchActive = isSearchActive,
|
isSearchActive = isSearchActive,
|
||||||
selectedRooms = selectedRooms,
|
selectedRooms = selectedRooms,
|
||||||
selectedAccount = selectedAccount,
|
selectedAccount = selectedAccount,
|
||||||
|
hasRoomListError = hasRoomListError,
|
||||||
otherAccounts = otherAccounts,
|
otherAccounts = otherAccounts,
|
||||||
isAccountListExpanded = isAccountListExpanded,
|
isAccountListExpanded = isAccountListExpanded,
|
||||||
eventSink = ::handleEvent,
|
eventSink = ::handleEvent,
|
||||||
|
|||||||
+4
@@ -23,6 +23,10 @@ data class RoomSelectState(
|
|||||||
val isSearchActive: Boolean,
|
val isSearchActive: Boolean,
|
||||||
val selectedRooms: ImmutableList<SelectRoomInfo>,
|
val selectedRooms: ImmutableList<SelectRoomInfo>,
|
||||||
val selectedAccount: MatrixUser,
|
val selectedAccount: MatrixUser,
|
||||||
|
/**
|
||||||
|
* True if the rooms of the selected account cannot be loaded.
|
||||||
|
*/
|
||||||
|
val hasRoomListError: Boolean,
|
||||||
val otherAccounts: ImmutableList<MatrixUser>,
|
val otherAccounts: ImmutableList<MatrixUser>,
|
||||||
val isAccountListExpanded: Boolean,
|
val isAccountListExpanded: Boolean,
|
||||||
val eventSink: (RoomSelectEvent) -> Unit,
|
val eventSink: (RoomSelectEvent) -> Unit,
|
||||||
|
|||||||
+8
@@ -62,6 +62,12 @@ open class RoomSelectStatePreviewParam : PreviewParameterProvider<RoomSelectStat
|
|||||||
otherAccounts = anOtherAccountList(),
|
otherAccounts = anOtherAccountList(),
|
||||||
isAccountListExpanded = true,
|
isAccountListExpanded = true,
|
||||||
),
|
),
|
||||||
|
aRoomSelectState(
|
||||||
|
mode = RoomSelectMode.Share,
|
||||||
|
selectedAccount = aMatrixUser(id = "@alice:example.org", displayName = "Alice"),
|
||||||
|
otherAccounts = anOtherAccountList(),
|
||||||
|
hasRoomListError = true,
|
||||||
|
),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -73,6 +79,7 @@ internal fun aRoomSelectState(
|
|||||||
isSearchActive: Boolean = false,
|
isSearchActive: Boolean = false,
|
||||||
selectedRooms: ImmutableList<SelectRoomInfo> = persistentListOf(),
|
selectedRooms: ImmutableList<SelectRoomInfo> = persistentListOf(),
|
||||||
selectedAccount: MatrixUser = MatrixUser(UserId("@alice:example.org")),
|
selectedAccount: MatrixUser = MatrixUser(UserId("@alice:example.org")),
|
||||||
|
hasRoomListError: Boolean = false,
|
||||||
otherAccounts: ImmutableList<MatrixUser> = persistentListOf(),
|
otherAccounts: ImmutableList<MatrixUser> = persistentListOf(),
|
||||||
isAccountListExpanded: Boolean = false,
|
isAccountListExpanded: Boolean = false,
|
||||||
eventSink: (RoomSelectEvent) -> Unit = {},
|
eventSink: (RoomSelectEvent) -> Unit = {},
|
||||||
@@ -84,6 +91,7 @@ internal fun aRoomSelectState(
|
|||||||
isSearchActive = isSearchActive,
|
isSearchActive = isSearchActive,
|
||||||
selectedRooms = selectedRooms,
|
selectedRooms = selectedRooms,
|
||||||
selectedAccount = selectedAccount,
|
selectedAccount = selectedAccount,
|
||||||
|
hasRoomListError = hasRoomListError,
|
||||||
otherAccounts = otherAccounts,
|
otherAccounts = otherAccounts,
|
||||||
isAccountListExpanded = isAccountListExpanded,
|
isAccountListExpanded = isAccountListExpanded,
|
||||||
eventSink = eventSink,
|
eventSink = eventSink,
|
||||||
|
|||||||
+17
-1
@@ -54,6 +54,8 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
|
|||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
import io.element.android.compound.theme.ElementTheme
|
import io.element.android.compound.theme.ElementTheme
|
||||||
import io.element.android.compound.tokens.generated.CompoundIcons
|
import io.element.android.compound.tokens.generated.CompoundIcons
|
||||||
|
import io.element.android.libraries.designsystem.atomic.molecules.IconTitleSubtitleMolecule
|
||||||
|
import io.element.android.libraries.designsystem.components.BigIcon
|
||||||
import io.element.android.libraries.designsystem.components.TopAppBarScrollBehaviorLayout
|
import io.element.android.libraries.designsystem.components.TopAppBarScrollBehaviorLayout
|
||||||
import io.element.android.libraries.designsystem.components.avatar.Avatar
|
import io.element.android.libraries.designsystem.components.avatar.Avatar
|
||||||
import io.element.android.libraries.designsystem.components.avatar.AvatarRow
|
import io.element.android.libraries.designsystem.components.avatar.AvatarRow
|
||||||
@@ -240,7 +242,9 @@ fun RoomSelectView(
|
|||||||
// showVerticalSpace only if there is no other accounts
|
// showVerticalSpace only if there is no other accounts
|
||||||
showVerticalSpace = !state.showAccountSwitch,
|
showVerticalSpace = !state.showAccountSwitch,
|
||||||
)
|
)
|
||||||
if (state.resultState is SearchBarResultState.Results) {
|
if (state.hasRoomListError) {
|
||||||
|
RoomListError()
|
||||||
|
} else if (state.resultState is SearchBarResultState.Results) {
|
||||||
LazyColumn(
|
LazyColumn(
|
||||||
state = lazyListState,
|
state = lazyListState,
|
||||||
contentPadding = lazyColumnContentPadding,
|
contentPadding = lazyColumnContentPadding,
|
||||||
@@ -270,6 +274,18 @@ fun RoomSelectView(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Composable
|
||||||
|
private fun RoomListError() {
|
||||||
|
IconTitleSubtitleMolecule(
|
||||||
|
modifier = Modifier
|
||||||
|
.fillMaxWidth()
|
||||||
|
.padding(horizontal = 24.dp, vertical = 32.dp),
|
||||||
|
title = stringResource(CommonStrings.common_something_went_wrong),
|
||||||
|
subTitle = stringResource(R.string.screen_room_select_error_cannot_load_rooms),
|
||||||
|
iconStyle = BigIcon.Style.AlertSolid,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Similar to the MultiAccountSection of the PreferencesRootView.
|
* Similar to the MultiAccountSection of the PreferencesRootView.
|
||||||
* Ref: https://www.figma.com/design/G1xy0HDZKJf5TCRFmKb5d5/Compound-Android-Components?node-id=5414-4759
|
* Ref: https://www.figma.com/design/G1xy0HDZKJf5TCRFmKb5d5/Compound-Android-Components?node-id=5414-4759
|
||||||
|
|||||||
@@ -0,0 +1,4 @@
|
|||||||
|
<?xml version="1.0" encoding="utf-8"?>
|
||||||
|
<resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
|
||||||
|
<string name="screen_room_select_error_cannot_load_rooms">"The room list associated with this account cannot be loaded."</string>
|
||||||
|
</resources>
|
||||||
+40
@@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.api.MatrixClientProvider
|
|||||||
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
|
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
|
||||||
import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
||||||
import io.element.android.libraries.matrix.api.user.MatrixUser
|
import io.element.android.libraries.matrix.api.user.MatrixUser
|
||||||
|
import io.element.android.libraries.matrix.test.AN_EXCEPTION
|
||||||
import io.element.android.libraries.matrix.test.A_ROOM_ID
|
import io.element.android.libraries.matrix.test.A_ROOM_ID
|
||||||
import io.element.android.libraries.matrix.test.A_ROOM_ID_2
|
import io.element.android.libraries.matrix.test.A_ROOM_ID_2
|
||||||
import io.element.android.libraries.matrix.test.A_SESSION_ID
|
import io.element.android.libraries.matrix.test.A_SESSION_ID
|
||||||
@@ -63,6 +64,45 @@ class RoomSelectPresenterTest {
|
|||||||
assertThat(initialState.otherAccounts).isEmpty()
|
assertThat(initialState.otherAccounts).isEmpty()
|
||||||
assertThat(initialState.isAccountListExpanded).isFalse()
|
assertThat(initialState.isAccountListExpanded).isFalse()
|
||||||
assertThat(initialState.showAccountSwitch).isFalse()
|
assertThat(initialState.showAccountSwitch).isFalse()
|
||||||
|
assertThat(initialState.hasRoomListError).isFalse()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - failure to get the client shows an error`() = runTest {
|
||||||
|
val presenter = createRoomSelectPresenter(
|
||||||
|
matrixClientProvider = FakeMatrixClientProvider { Result.failure(AN_EXCEPTION) },
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val state = consumeItemsUntilPredicate { it.hasRoomListError }.last()
|
||||||
|
assertThat(state.hasRoomListError).isTrue()
|
||||||
|
assertThat(state.resultState).isInstanceOf(SearchBarResultState.Initial::class.java)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - select another account whose client cannot be restored shows an error`() = runTest {
|
||||||
|
val presenter = createRoomSelectPresenter(
|
||||||
|
mode = RoomSelectMode.Share,
|
||||||
|
sessionStore = aSessionStoreWithTwoAccounts(),
|
||||||
|
matrixClientProvider = FakeMatrixClientProvider { sessionId ->
|
||||||
|
if (sessionId == A_SESSION_ID) {
|
||||||
|
Result.success(FakeMatrixClient(sessionId = sessionId))
|
||||||
|
} else {
|
||||||
|
Result.failure(AN_EXCEPTION)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val state = awaitLastSequentialItem()
|
||||||
|
assertThat(state.hasRoomListError).isFalse()
|
||||||
|
state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2))
|
||||||
|
val errorState = consumeItemsUntilPredicate { it.hasRoomListError }.last()
|
||||||
|
assertThat(errorState.selectedAccount.userId).isEqualTo(A_SESSION_ID_2)
|
||||||
|
// Going back to the first account clears the error
|
||||||
|
errorState.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID))
|
||||||
|
val finalState = consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID && !it.hasRoomListError }.last()
|
||||||
|
assertThat(finalState.hasRoomListError).isFalse()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+13
@@ -122,6 +122,19 @@ class RoomSelectViewTest : RobolectricTest() {
|
|||||||
)
|
)
|
||||||
onNodeWithText(activity!!.getString(CommonStrings.common_switch_account)).assertDoesNotExist()
|
onNodeWithText(activity!!.getString(CommonStrings.common_switch_account)).assertDoesNotExist()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an error is displayed instead of the rooms if they cannot be loaded`() = runAndroidComposeUiTest<ComponentActivity> {
|
||||||
|
setRoomSelectView(
|
||||||
|
aRoomSelectState(
|
||||||
|
resultState = SearchBarResultState.Results(aRoomSelectRoomList()),
|
||||||
|
hasRoomListError = true,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
onNodeWithText(activity!!.getString(CommonStrings.common_something_went_wrong)).assertIsDisplayed()
|
||||||
|
onNodeWithText(activity!!.getString(R.string.screen_room_select_error_cannot_load_rooms)).assertIsDisplayed()
|
||||||
|
onNodeWithText("Room with alias").assertDoesNotExist()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun AndroidComposeUiTest<ComponentActivity>.setRoomSelectView(
|
private fun AndroidComposeUiTest<ComponentActivity>.setRoomSelectView(
|
||||||
|
|||||||
@@ -14,6 +14,12 @@
|
|||||||
"screen_bug_report_.*"
|
"screen_bug_report_.*"
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"name" : ":libraries:roomselect:impl",
|
||||||
|
"includeRegex" : [
|
||||||
|
"screen\\.room_select\\..*"
|
||||||
|
]
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"name" : ":features:rageshake:api",
|
"name" : ":features:rageshake:api",
|
||||||
"includeRegex" : [
|
"includeRegex" : [
|
||||||
|
|||||||
Reference in New Issue
Block a user