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

#4319: Fix DM navigation in member profile screen #5292

Merged
merged 4 commits into from
Feb 28, 2022
Merged

#4319: Fix DM navigation in member profile screen #5292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ef26e6
Adding changelog entry
Feb 21, 2022
836bf4e
DRAFT TODO
Feb 21, 2022
46854b4
Moving DM creation/opening into the member profile screen
Feb 21, 2022
96f041a
Remove handling of direct message from timeline fragment
Feb 22, 2022
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 changelog.d/4319.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Open direct message screen when clicking on DM button in the space members list
1 change: 0 additions & 1 deletion vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ sealed class RoomDetailAction : VectorViewModelAction {

data class UpdateJoinJitsiCallStatus(val conferenceEvent: ConferenceEvent) : RoomDetailAction()

data class OpenOrCreateDm(val userId: String) : RoomDetailAction()
data class JumpToReadReceipt(val userId: String) : RoomDetailAction()
object QuickActionInvitePeople : RoomDetailAction()
object QuickActionSetAvatar : RoomDetailAction()
Expand Down
1 change: 0 additions & 1 deletion vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailPendingAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package im.vector.app.features.home.room.detail

sealed class RoomDetailPendingAction {
data class OpenOrCreateDm(val userId: String) : RoomDetailPendingAction()
data class JumpToReadReceipt(val userId: String) : RoomDetailPendingAction()
data class MentionUser(val userId: String) : RoomDetailPendingAction()
data class OpenRoom(val roomId: String, val closeCurrentRoom: Boolean = false) : RoomDetailPendingAction()
Expand Down
2 changes: 0 additions & 2 deletions vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1247,8 +1247,6 @@ class TimelineFragment @Inject constructor(
timelineViewModel.handle(RoomDetailAction.JumpToReadReceipt(roomDetailPendingAction.userId))
is RoomDetailPendingAction.MentionUser ->
insertUserDisplayNameInTextEditor(roomDetailPendingAction.userId)
is RoomDetailPendingAction.OpenOrCreateDm ->
timelineViewModel.handle(RoomDetailAction.OpenOrCreateDm(roomDetailPendingAction.userId))
is RoomDetailPendingAction.OpenRoom ->
handleOpenRoom(RoomDetailViewEvents.OpenRoom(roomDetailPendingAction.roomId, roomDetailPendingAction.closeCurrentRoom))
}.exhaustive
Expand Down
17 changes: 1 addition & 16 deletions vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ class TimelineViewModel @AssistedInject constructor(
is RoomDetailAction.RemoveWidget -> handleDeleteWidget(action.widgetId)
is RoomDetailAction.EnsureNativeWidgetAllowed -> handleCheckWidgetAllowed(action)
is RoomDetailAction.CancelSend -> handleCancel(action)
is RoomDetailAction.OpenOrCreateDm -> handleOpenOrCreateDm(action)
is RoomDetailAction.JumpToReadReceipt -> handleJumpToReadReceipt(action)
RoomDetailAction.QuickActionInvitePeople -> handleInvitePeople()
RoomDetailAction.QuickActionSetAvatar -> handleQuickSetAvatar()
Expand Down Expand Up @@ -497,20 +496,6 @@ class TimelineViewModel @AssistedInject constructor(
_viewEvents.post(RoomDetailViewEvents.OpenSetRoomAvatarDialog)
}

private fun handleOpenOrCreateDm(action: RoomDetailAction.OpenOrCreateDm) {
viewModelScope.launch {
val roomId = try {
directRoomHelper.ensureDMExists(action.userId)
} catch (failure: Throwable) {
_viewEvents.post(RoomDetailViewEvents.ActionFailure(action, failure))
return@launch
}
if (roomId != initialState.roomId) {
_viewEvents.post(RoomDetailViewEvents.OpenRoom(roomId = roomId))
}
}
}

private fun handleJumpToReadReceipt(action: RoomDetailAction.JumpToReadReceipt) {
room.getUserReadReceipt(action.userId)
?.let { handleNavigateToEvent(RoomDetailAction.NavigateToEvent(it, true)) }
Expand Down Expand Up @@ -810,7 +795,7 @@ class TimelineViewModel @AssistedInject constructor(
notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(initialState.roomId) }
viewModelScope.launch {
try {
session.leaveRoom(room.roomId)
session.leaveRoom(room.roomId)
} catch (throwable: Throwable) {
_viewEvents.post(RoomDetailViewEvents.Failure(throwable, showInDialog = true))
}
Expand Down
1 change: 1 addition & 0 deletions vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ sealed class RoomMemberProfileAction : VectorViewModelAction {
object ShareRoomMemberProfile : RoomMemberProfileAction()
data class SetPowerLevel(val previousValue: Int, val newValue: Int, val askForValidation: Boolean) : RoomMemberProfileAction()
data class SetUserColorOverride(val newColorSpec: String) : RoomMemberProfileAction()
data class OpenOrCreateDm(val userId: String) : RoomMemberProfileAction()
}
8 changes: 6 additions & 2 deletions vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class RoomMemberProfileFragment @Inject constructor(
is RoomMemberProfileViewEvents.ShareRoomMemberProfile -> handleShareRoomMemberProfile(it.permalink)
is RoomMemberProfileViewEvents.ShowPowerLevelValidation -> handleShowPowerLevelAdminWarning(it)
is RoomMemberProfileViewEvents.ShowPowerLevelDemoteWarning -> handleShowPowerLevelDemoteWarning(it)
is RoomMemberProfileViewEvents.OpenRoom -> handleOpenRoom(it)
is RoomMemberProfileViewEvents.OnKickActionSuccess -> Unit
is RoomMemberProfileViewEvents.OnSetPowerLevelSuccess -> Unit
is RoomMemberProfileViewEvents.OnBanActionSuccess -> Unit
Expand All @@ -142,6 +143,10 @@ class RoomMemberProfileFragment @Inject constructor(
headerViews.memberProfileIdView.copyOnLongClick()
}

private fun handleOpenRoom(event: RoomMemberProfileViewEvents.OpenRoom) {
navigator.openRoom(requireContext(), event.roomId, null)
}

private fun handleShowPowerLevelDemoteWarning(event: RoomMemberProfileViewEvents.ShowPowerLevelDemoteWarning) {
EditPowerLevelDialogs.showDemoteWarning(requireActivity()) {
viewModel.handle(RoomMemberProfileAction.SetPowerLevel(event.currentValue, event.newValue, false))
Expand Down Expand Up @@ -297,8 +302,7 @@ class RoomMemberProfileFragment @Inject constructor(
}

override fun onOpenDmClicked() {
roomDetailPendingActionStore.data = RoomDetailPendingAction.OpenOrCreateDm(fragmentArgs.userId)
vectorBaseActivity.finish()
viewModel.handle(RoomMemberProfileAction.OpenOrCreateDm(fragmentArgs.userId))
Copy link
Member

Choose a reason for hiding this comment

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

This has the effect to add a room to the Activity stack. This is fine, but this is a side effect of this change.
So when the user will close the RoomDetailActivity with the DM, they will see the RoomMemberProfile Fragment again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes It was intended. In my opinion it is consistent to go back to member profile screen in term of navigation. But I will try to make a video to explain the changes as you suggested.

}

override fun onJumpToReadReceiptClicked() {
Expand Down
1 change: 1 addition & 0 deletions vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileViewEvents.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ sealed class RoomMemberProfileViewEvents : VectorViewEvents {
) : RoomMemberProfileViewEvents()

data class ShareRoomMemberProfile(val permalink: String) : RoomMemberProfileViewEvents()
data class OpenRoom(val roomId: String) : RoomMemberProfileViewEvents()
}
18 changes: 18 additions & 0 deletions vector/src/main/java/im/vector/app/features/roommemberprofile/RoomMemberProfileViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import im.vector.app.core.extensions.exhaustive
import im.vector.app.core.mvrx.runCatchingToAsync
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.core.resources.StringProvider
import im.vector.app.features.createdirect.DirectRoomHelper
import im.vector.app.features.displayname.getBestName
import im.vector.app.features.home.room.detail.timeline.helper.MatrixItemColorProvider
import im.vector.app.features.powerlevel.PowerLevelsFlowFactory
Expand Down Expand Up @@ -66,6 +67,7 @@ class RoomMemberProfileViewModel @AssistedInject constructor(
@Assisted private val initialState: RoomMemberProfileViewState,
private val stringProvider: StringProvider,
private val matrixItemColorProvider: MatrixItemColorProvider,
private val directRoomHelper: DirectRoomHelper,
private val session: Session
) : VectorViewModel<RoomMemberProfileViewState, RoomMemberProfileAction, RoomMemberProfileViewEvents>(initialState) {

Expand Down Expand Up @@ -167,9 +169,25 @@ class RoomMemberProfileViewModel @AssistedInject constructor(
is RoomMemberProfileAction.KickUser -> handleKickAction(action)
RoomMemberProfileAction.InviteUser -> handleInviteAction()
is RoomMemberProfileAction.SetUserColorOverride -> handleSetUserColorOverride(action)
is RoomMemberProfileAction.OpenOrCreateDm -> handleOpenOrCreateDm(action)
}.exhaustive
}

private fun handleOpenOrCreateDm(action: RoomMemberProfileAction.OpenOrCreateDm) {
viewModelScope.launch {
_viewEvents.post(RoomMemberProfileViewEvents.Loading())
val roomId = try {
directRoomHelper.ensureDMExists(action.userId)
} catch (failure: Throwable) {
_viewEvents.post(RoomMemberProfileViewEvents.Failure(failure))
return@launch
}
if (roomId != initialState.roomId) {
_viewEvents.post(RoomMemberProfileViewEvents.OpenRoom(roomId = roomId))
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This code is duplicate of the code in TimelineViewModel.
We should find a way to avoid code duplication in ViewModels (maybe out of scope of this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the code which is duplicated? Normally, I moved out this method handleOpenOrCreateDm from TimelineViewModel into the RoomMemberProfileViewModel.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, my bad, sorry


private fun handleSetUserColorOverride(action: RoomMemberProfileAction.SetUserColorOverride) {
val newOverrideColorSpecs = session.accountDataService()
.getUserAccountDataEvent(UserAccountDataTypes.TYPE_OVERRIDE_COLORS)
Expand Down