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

Improve invite bottom sheet #4057

Merged
merged 4 commits into from
Sep 24, 2021
Merged

Improve invite bottom sheet #4057

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
195fb6f
Improve invite bottom sheet
BillCarsonFr Sep 22, 2021
9b02704
Code review
BillCarsonFr Sep 23, 2021
3287980
Cleanup and add changelog
bmarty Sep 24, 2021
eb104d8
Clean up
bmarty Sep 24, 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
38 changes: 37 additions & 1 deletion vector/src/main/java/im/vector/app/features/spaces/invite/SpaceInviteBottomSheetViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package im.vector.app.features.spaces.invite

import androidx.lifecycle.viewModelScope
import com.airbnb.mvrx.ActivityViewModelContext
import com.airbnb.mvrx.Fail
import com.airbnb.mvrx.FragmentViewModelContext
Expand All @@ -32,7 +33,11 @@ import im.vector.app.core.platform.VectorViewModel
import im.vector.app.features.session.coroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.room.peeking.PeekResult

class SpaceInviteBottomSheetViewModel @AssistedInject constructor(
@Assisted private val initialState: SpaceInviteBottomSheetState,
Expand All @@ -42,7 +47,6 @@ class SpaceInviteBottomSheetViewModel @AssistedInject constructor(

init {
session.getRoomSummary(initialState.spaceId)?.let { roomSummary ->

val knownMembers = roomSummary.otherMemberIds.filter {
session.getExistingDirectRoomWithUser(it) != null
}.mapNotNull { session.getUser(it) }
Expand All @@ -57,6 +61,38 @@ class SpaceInviteBottomSheetViewModel @AssistedInject constructor(
peopleYouKnow = Success(peopleYouKnow)
)
}
refreshInviteSummaryIfNeeded(roomSummary)
}
}

private fun refreshInviteSummaryIfNeeded(roomSummary: RoomSummary) {
if (roomSummary.membership == Membership.INVITE) {
Copy link
Member

Choose a reason for hiding this comment

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

To follow @ganfra 's remark, I would have let this if test at the calling place (i.e. in the init {} block) to reduce indentation in the body of this method. Also the method could then be renamed to fetchRoomSummary

// we can try to query the room summary api to get more info?
Copy link
Member

Choose a reason for hiding this comment

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

Remove the trailing ??

viewModelScope.launch(Dispatchers.IO) {
when (val peekResult = tryOrNull { session.peekRoom(roomSummary.roomId) }) {
is PeekResult.Success -> {
setState {
copy(
summary = Success(
roomSummary.copy(
joinedMembersCount = peekResult.numJoinedMembers,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe have a small method to map PeekResult to RoomSummary also?

Copy link
Member

Choose a reason for hiding this comment

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

Here we are copying an existing roomSummary with info from peekResult, so I think this is not applicable

// it's also possible that the name/avatar did change since the invite..
// if it's null keep the old one as summary API might not be available
// and peek result could be null for other reasons (not peekable)
avatarUrl = peekResult.avatarUrl ?: roomSummary.avatarUrl,
displayName = peekResult.name ?: roomSummary.displayName,
topic = peekResult.topic ?: roomSummary.topic
// maybe use someMembers field later?
)
)
)
}
}
else -> {
// nop
}
}
}
}
}

Expand Down