Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error feedback when joining space rooms #3514

Merged
merged 2 commits into from
Jun 24, 2021
Merged

Add error feedback when joining space rooms #3514

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e277dee
Add error feedback when joining space rooms
BillCarsonFr Jun 16, 2021
2a675bf
Code review
BillCarsonFr Jun 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragment/3207.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Space Explore Rooms no feedback on failed to join
16 changes: 14 additions & 2 deletions vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
package im.vector.app.features.home.room.list

import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Fail
import com.airbnb.mvrx.Loading
import im.vector.app.R
import im.vector.app.core.date.DateFormatKind
import im.vector.app.core.date.VectorDateFormatter
import im.vector.app.core.epoxy.VectorEpoxyModel
import im.vector.app.core.error.ErrorFormatter
import im.vector.app.core.resources.StringProvider
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.home.room.detail.timeline.format.DisplayableEventFormatter
Expand All @@ -37,7 +39,8 @@ class RoomSummaryItemFactory @Inject constructor(private val displayableEventFor
private val dateFormatter: VectorDateFormatter,
private val stringProvider: StringProvider,
private val typingHelper: TypingHelper,
private val avatarRenderer: AvatarRenderer) {
private val avatarRenderer: AvatarRenderer,
private val errorFormatter: ErrorFormatter) {

fun create(roomSummary: RoomSummary,
roomChangeMembershipStates: Map<String, ChangeMembershipState>,
Expand All @@ -55,12 +58,21 @@ class RoomSummaryItemFactory @Inject constructor(private val displayableEventFor
fun createSuggestion(spaceChildInfo: SpaceChildInfo,
suggestedRoomJoiningStates: Map<String, Async<Unit>>,
listener: RoomListListener?): VectorEpoxyModel<*> {
val error = (suggestedRoomJoiningStates[spaceChildInfo.childRoomId] as? Fail)?.error
return SpaceChildInfoItem_()
.id("sug_${spaceChildInfo.childRoomId}")
.matrixItem(spaceChildInfo.toMatrixItem())
.avatarRenderer(avatarRenderer)
.topic(spaceChildInfo.topic)
.buttonLabel(stringProvider.getString(R.string.join))
.errorLabel(
error?.let {
stringProvider.getString(R.string.error_failed_to_join_room, errorFormatter.toHumanReadable(it))
}
bmarty marked this conversation as resolved.
Show resolved Hide resolved
)
.buttonLabel(
if (error != null) stringProvider.getString(R.string.global_retry)
else stringProvider.getString(R.string.join)
)
.loading(suggestedRoomJoiningStates[spaceChildInfo.childRoomId] is Loading)
.memberCount(spaceChildInfo.activeMemberCount ?: 0)
.buttonClickListener { listener?.onJoinSuggestedRoom(spaceChildInfo) }
Expand Down
5 changes: 5 additions & 0 deletions vector/src/main/java/im/vector/app/features/home/room/list/SpaceChildInfoItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import im.vector.app.core.epoxy.ClickListener
import im.vector.app.core.epoxy.VectorEpoxyHolder
import im.vector.app.core.epoxy.VectorEpoxyModel
import im.vector.app.core.epoxy.onClick
import im.vector.app.core.extensions.setTextOrHide
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.themes.ThemeUtils
import me.gujun.android.span.image
Expand All @@ -52,6 +53,7 @@ abstract class SpaceChildInfoItem : VectorEpoxyModel<SpaceChildInfoItem.Holder>(
@EpoxyAttribute var loading: Boolean = false

@EpoxyAttribute var buttonLabel: String? = null
@EpoxyAttribute var errorLabel: String? = null

@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var itemLongClickListener: View.OnLongClickListener? = null
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var itemClickListener: ClickListener? = null
Expand Down Expand Up @@ -97,6 +99,8 @@ abstract class SpaceChildInfoItem : VectorEpoxyModel<SpaceChildInfoItem.Holder>(
holder.joinButton.isVisible = true
}

holder.errorTextView.setTextOrHide(errorLabel)

holder.joinButton.onClick {
// local echo
holder.joinButton.isEnabled = false
Expand All @@ -120,5 +124,6 @@ abstract class SpaceChildInfoItem : VectorEpoxyModel<SpaceChildInfoItem.Holder>(
val descriptionText by bind<TextView>(R.id.suggestedRoomDescription)
val avatarImageView by bind<ImageView>(R.id.roomAvatarImageView)
val rootView by bind<ViewGroup>(R.id.itemRoomLayout)
val errorTextView by bind<TextView>(R.id.inlineErrorText)
}
}
14 changes: 12 additions & 2 deletions vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import im.vector.app.features.home.room.list.spaceChildInfoItem
import me.gujun.android.span.span
import org.matrix.android.sdk.api.failure.Failure
import org.matrix.android.sdk.api.failure.MatrixError.Companion.M_UNRECOGNIZED
import org.matrix.android.sdk.api.session.room.members.ChangeMembershipState
import org.matrix.android.sdk.api.session.room.model.RoomType
import org.matrix.android.sdk.api.session.room.model.SpaceChildInfo
import org.matrix.android.sdk.api.util.toMatrixItem
Expand Down Expand Up @@ -127,6 +128,7 @@ class SpaceDirectoryController @Inject constructor(
val isSpace = info.roomType == RoomType.SPACE
val isJoined = data?.joinedRoomsIds?.contains(info.childRoomId) == true
val isLoading = data?.changeMembershipStates?.get(info.childRoomId)?.isInProgress() ?: false
val error = (data?.changeMembershipStates?.get(info.childRoomId) as? ChangeMembershipState.FailedJoining)?.throwable
// if it's known use that matrixItem because it would have a better computed name
val matrixItem = data?.knownRoomSummaries?.find { it.roomId == info.childRoomId }?.toMatrixItem()
?: info.toMatrixItem()
Expand All @@ -135,11 +137,19 @@ class SpaceDirectoryController @Inject constructor(
matrixItem(matrixItem)
avatarRenderer(host.avatarRenderer)
topic(info.topic)
errorLabel(
error?.let {
host.stringProvider.getString(R.string.error_failed_to_join_room, host.errorFormatter.toHumanReadable(it))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same remark

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I can't, if null is past the error formatter returns "Sorry, an error occurred"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad

)
memberCount(info.activeMemberCount ?: 0)
loading(isLoading)
buttonLabel(
if (isJoined) host.stringProvider.getString(R.string.action_open)
else host.stringProvider.getString(R.string.join)
when {
error != null -> host.stringProvider.getString(R.string.global_retry)
isJoined -> host.stringProvider.getString(R.string.action_open)
else -> host.stringProvider.getString(R.string.join)
}
)
apply {
if (isSpace) {
Expand Down
9 changes: 5 additions & 4 deletions vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,20 +188,21 @@ class SpaceDirectoryViewModel @AssistedInject constructor(

private fun handleJoinOrOpen(spaceChildInfo: SpaceChildInfo) = withState { state ->
val isSpace = spaceChildInfo.roomType == RoomType.SPACE
if (state.joinedRoomsIds.contains(spaceChildInfo.childRoomId)) {
val childId = spaceChildInfo.childRoomId
if (state.joinedRoomsIds.contains(childId)) {
if (isSpace) {
handle(SpaceDirectoryViewAction.ExploreSubSpace(spaceChildInfo))
} else {
_viewEvents.post(SpaceDirectoryViewEvents.NavigateToRoom(spaceChildInfo.childRoomId))
_viewEvents.post(SpaceDirectoryViewEvents.NavigateToRoom(childId))
}
} else {
// join
viewModelScope.launch {
try {
if (isSpace) {
session.spaceService().joinSpace(spaceChildInfo.childRoomId, null, spaceChildInfo.viaServers)
session.spaceService().joinSpace(childId, null, spaceChildInfo.viaServers)
} else {
session.joinRoom(spaceChildInfo.childRoomId, null, spaceChildInfo.viaServers)
session.joinRoom(childId, null, spaceChildInfo.viaServers)
}
} catch (failure: Throwable) {
Timber.e(failure, "## Space: Failed to join room or subspace")
Expand Down
15 changes: 13 additions & 2 deletions vector/src/main/res/layout/item_suggested_room.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
android:layout_marginEnd="8dp"
android:maxWidth="@dimen/button_max_width"
android:text="@string/join"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintBottom_toTopOf="@id/inlineErrorText"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent" />

Expand All @@ -102,6 +102,17 @@
app:barrierDirection="bottom"
app:constraint_referenced_ids="roomAvatarBottomSpace" />

<TextView
bmarty marked this conversation as resolved.
Show resolved Hide resolved
android:id="@+id/inlineErrorText"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:drawablePadding="8dp"
android:padding="8dp"
android:textColor="@color/design_default_color_error"
bmarty marked this conversation as resolved.
Show resolved Hide resolved
app:drawableStartCompat="@drawable/ic_warning_badge"
app:layout_constraintTop_toBottomOf="@+id/roomBottomBarrier"
tools:text="An error occured while joining" />

<!-- We use vctr_list_separator_system here for a better rendering -->
<View
android:id="@+id/roomDividerView"
Expand All @@ -111,6 +122,6 @@
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/roomBottomBarrier" />
app:layout_constraintTop_toBottomOf="@+id/inlineErrorText" />

</androidx.constraintlayout.widget.ConstraintLayout>
1 change: 1 addition & 0 deletions vector/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3403,4 +3403,5 @@

<string name="teammate_spaces_arent_quite_ready">"Teammate spaces aren’t quite ready but you can still give them a try"</string>
<string name="teammate_spaces_might_not_join">"At the moment people might not be able to join any private rooms you make.\n\nWe’ll be improving this as part of the beta, but just wanted to let you know."</string>
<string name="error_failed_to_join_room">Sorry, an error occurred while trying to join: %s</string>
</resources>