From 5e2c3239eabb99bf761829abf3f56026ef228e8d Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 10 May 2021 10:21:43 +0200 Subject: [PATCH] Code review --- .../session/space/DefaultSpaceService.kt | 31 ++++++++++--------- .../features/matrixto/MatrixToBottomSheet.kt | 9 +++++- .../matrixto/MatrixToRoomSpaceFragment.kt | 22 +++++++------ .../features/spaces/SpaceExploreActivity.kt | 18 ++++++++++- .../spaces/explore/SpaceDirectoryFragment.kt | 25 +++------------ .../explore/SpaceDirectoryViewAction.kt | 1 + .../explore/SpaceDirectoryViewEvents.kt | 1 + .../spaces/explore/SpaceDirectoryViewModel.kt | 9 +++++- 8 files changed, 69 insertions(+), 47 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt index 540eed9b6a..fb6a54e3a8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt @@ -130,23 +130,24 @@ internal class DefaultSpaceService @Inject constructor( ?.flatMap { childSummary -> response.events ?.filter { it.stateKey == childSummary.roomId && it.type == EventType.STATE_SPACE_CHILD } - ?.map { childStateEv -> + ?.mapNotNull { childStateEv -> // create a child entry for everytime this room is the child of a space // beware that a room could appear then twice in this list - val childStateEvContent = childStateEv.content.toModel() - SpaceChildInfo( - childRoomId = childSummary.roomId, - isKnown = true, - roomType = childSummary.roomType, - name = childSummary.name, - topic = childSummary.topic, - avatarUrl = childSummary.avatarUrl, - order = childStateEvContent?.order, - autoJoin = childStateEvContent?.autoJoin ?: false, - viaServers = childStateEvContent?.via ?: emptyList(), - activeMemberCount = childSummary.numJoinedMembers, - parentRoomId = childStateEv.roomId - ) + childStateEv.content.toModel()?.let { childStateEvContent -> + SpaceChildInfo( + childRoomId = childSummary.roomId, + isKnown = true, + roomType = childSummary.roomType, + name = childSummary.name, + topic = childSummary.topic, + avatarUrl = childSummary.avatarUrl, + order = childStateEvContent?.order, + autoJoin = childStateEvContent?.autoJoin ?: false, + viaServers = childStateEvContent?.via ?: emptyList(), + activeMemberCount = childSummary.numJoinedMembers, + parentRoomId = childStateEv.roomId + ) + } }.orEmpty() } .orEmpty() diff --git a/vector/src/main/java/im/vector/app/features/matrixto/MatrixToBottomSheet.kt b/vector/src/main/java/im/vector/app/features/matrixto/MatrixToBottomSheet.kt index 13c0645d8d..aec52a571c 100644 --- a/vector/src/main/java/im/vector/app/features/matrixto/MatrixToBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/matrixto/MatrixToBottomSheet.kt @@ -36,6 +36,7 @@ import im.vector.app.databinding.BottomSheetMatrixToCardBinding import im.vector.app.features.home.AvatarRenderer import kotlinx.parcelize.Parcelize import org.matrix.android.sdk.api.session.permalinks.PermalinkData +import java.lang.ref.WeakReference import javax.inject.Inject import kotlin.reflect.KClass @@ -56,7 +57,13 @@ class MatrixToBottomSheet : injector.inject(this) } - var interactionListener: InteractionListener? = null + var weakReference = WeakReference(null) + + var interactionListener: InteractionListener? + set(value) { + weakReference = WeakReference(value) + } + get() = weakReference.get() override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): BottomSheetMatrixToCardBinding { return BottomSheetMatrixToCardBinding.inflate(inflater, container, false) diff --git a/vector/src/main/java/im/vector/app/features/matrixto/MatrixToRoomSpaceFragment.kt b/vector/src/main/java/im/vector/app/features/matrixto/MatrixToRoomSpaceFragment.kt index 43fd819c4d..04f72000fa 100644 --- a/vector/src/main/java/im/vector/app/features/matrixto/MatrixToRoomSpaceFragment.kt +++ b/vector/src/main/java/im/vector/app/features/matrixto/MatrixToRoomSpaceFragment.kt @@ -171,16 +171,20 @@ class MatrixToRoomSpaceFragment @Inject constructor( when (state.peopleYouKnow) { is Success -> { val someYouKnow = state.peopleYouKnow.invoke() - someYouKnow.forEachIndexed { index, item -> - images[index].isVisible = true - avatarRenderer.render(item, images[index]) + if (someYouKnow.isEmpty()) { + views.peopleYouMayKnowText.isVisible = false + } else { + someYouKnow.forEachIndexed { index, item -> + images[index].isVisible = true + avatarRenderer.render(item, images[index]) + } + views.peopleYouMayKnowText.setTextOrHide( + resources.getQuantityString(R.plurals.space_people_you_know, + someYouKnow.count(), + someYouKnow.count() + ) + ) } - views.peopleYouMayKnowText.setTextOrHide( - resources.getQuantityString(R.plurals.space_people_you_know, - someYouKnow.count(), - someYouKnow.count() - ) - ) } else -> { views.peopleYouMayKnowText.isVisible = false diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceExploreActivity.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceExploreActivity.kt index e07896de50..3a5fd883f3 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceExploreActivity.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceExploreActivity.kt @@ -19,6 +19,7 @@ package im.vector.app.features.spaces import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.fragment.app.Fragment import com.airbnb.mvrx.MvRx import com.airbnb.mvrx.viewModel import im.vector.app.R @@ -26,6 +27,7 @@ import im.vector.app.core.di.ScreenComponent import im.vector.app.core.extensions.commitTransaction import im.vector.app.core.platform.VectorBaseActivity import im.vector.app.databinding.ActivitySimpleBinding +import im.vector.app.features.matrixto.MatrixToBottomSheet import im.vector.app.features.spaces.explore.SpaceDirectoryArgs import im.vector.app.features.spaces.explore.SpaceDirectoryFragment import im.vector.app.features.spaces.explore.SpaceDirectoryState @@ -33,7 +35,7 @@ import im.vector.app.features.spaces.explore.SpaceDirectoryViewEvents import im.vector.app.features.spaces.explore.SpaceDirectoryViewModel import javax.inject.Inject -class SpaceExploreActivity : VectorBaseActivity(), SpaceDirectoryViewModel.Factory { +class SpaceExploreActivity : VectorBaseActivity(), SpaceDirectoryViewModel.Factory, MatrixToBottomSheet.InteractionListener { @Inject lateinit var spaceDirectoryViewModelFactory: SpaceDirectoryViewModel.Factory @@ -72,10 +74,20 @@ class SpaceExploreActivity : VectorBaseActivity(), SpaceD is SpaceDirectoryViewEvents.NavigateToRoom -> { navigator.openRoom(this, it.roomId) } + is SpaceDirectoryViewEvents.NavigateToMxToBottomSheet -> { + MatrixToBottomSheet.withLink(it.link, this).show(supportFragmentManager, "ShowChild") + } } } } + override fun onAttachFragment(fragment: Fragment) { + if (fragment is MatrixToBottomSheet) { + fragment.interactionListener = this + } + super.onAttachFragment(fragment) + } + companion object { fun newIntent(context: Context, spaceId: String): Intent { return Intent(context, SpaceExploreActivity::class.java).apply { @@ -86,4 +98,8 @@ class SpaceExploreActivity : VectorBaseActivity(), SpaceD override fun create(initialState: SpaceDirectoryState): SpaceDirectoryViewModel = spaceDirectoryViewModelFactory.create(initialState) + + override fun navigateToRoom(roomId: String) { + navigator.openRoom(this, roomId) + } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryFragment.kt b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryFragment.kt index 0454112417..41ab8d9006 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryFragment.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryFragment.kt @@ -21,7 +21,6 @@ import android.os.Parcelable import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import androidx.fragment.app.FragmentOnAttachListener import com.airbnb.mvrx.activityViewModel import com.airbnb.mvrx.withState import im.vector.app.R @@ -30,7 +29,6 @@ import im.vector.app.core.extensions.configureWith import im.vector.app.core.platform.OnBackPressed import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.databinding.FragmentRoomDirectoryPickerBinding -import im.vector.app.features.matrixto.MatrixToBottomSheet import kotlinx.parcelize.Parcelize import org.matrix.android.sdk.api.session.room.model.SpaceChildInfo import javax.inject.Inject @@ -44,19 +42,13 @@ class SpaceDirectoryFragment @Inject constructor( private val epoxyController: SpaceDirectoryController ) : VectorBaseFragment(), SpaceDirectoryController.InteractionListener, - OnBackPressed, MatrixToBottomSheet.InteractionListener { + OnBackPressed { override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = FragmentRoomDirectoryPickerBinding.inflate(layoutInflater, container, false) private val viewModel by activityViewModel(SpaceDirectoryViewModel::class) - private var fragmentOnAttachListener = FragmentOnAttachListener { _, fragment -> - if (fragment is MatrixToBottomSheet) { - fragment.interactionListener = this - } - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) @@ -68,12 +60,9 @@ class SpaceDirectoryFragment @Inject constructor( } epoxyController.listener = this views.roomDirectoryPickerList.configureWith(epoxyController) - - childFragmentManager.addFragmentOnAttachListener(fragmentOnAttachListener) } override fun onDestroyView() { - childFragmentManager.removeFragmentOnAttachListener(fragmentOnAttachListener) epoxyController.listener = null views.roomDirectoryPickerList.cleanup() super.onDestroyView() @@ -97,11 +86,7 @@ class SpaceDirectoryFragment @Inject constructor( } override fun onRoomClick(spaceChildInfo: SpaceChildInfo) { - // This is temporary for now to at least display something for the space beta - // It's not ideal as it's doing some peeking that is not needed. - viewModel.session.permalinkService().createRoomPermalink(spaceChildInfo.childRoomId)?.let { - MatrixToBottomSheet.withLink(it, this).show(childFragmentManager, "ShowChild") - } + viewModel.handle(SpaceDirectoryViewAction.ShowDetails(spaceChildInfo)) } override fun onBackPressed(toolbarButton: Boolean): Boolean { @@ -109,7 +94,7 @@ class SpaceDirectoryFragment @Inject constructor( return true } - override fun navigateToRoom(roomId: String) { - viewModel.handle(SpaceDirectoryViewAction.NavigateToRoom(roomId)) - } +// override fun navigateToRoom(roomId: String) { +// viewModel.handle(SpaceDirectoryViewAction.NavigateToRoom(roomId)) +// } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewAction.kt b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewAction.kt index 562d1c1124..a7e1482e24 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewAction.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewAction.kt @@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.room.model.SpaceChildInfo sealed class SpaceDirectoryViewAction : VectorViewModelAction { data class ExploreSubSpace(val spaceChildInfo: SpaceChildInfo) : SpaceDirectoryViewAction() data class JoinOrOpen(val spaceChildInfo: SpaceChildInfo) : SpaceDirectoryViewAction() + data class ShowDetails(val spaceChildInfo: SpaceChildInfo) : SpaceDirectoryViewAction() data class NavigateToRoom(val roomId: String) : SpaceDirectoryViewAction() object HandleBack : SpaceDirectoryViewAction() } diff --git a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewEvents.kt b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewEvents.kt index ef6f5385d3..3ac0426de9 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewEvents.kt @@ -21,4 +21,5 @@ import im.vector.app.core.platform.VectorViewEvents sealed class SpaceDirectoryViewEvents : VectorViewEvents { object Dismiss : SpaceDirectoryViewEvents() data class NavigateToRoom(val roomId: String) : SpaceDirectoryViewEvents() + data class NavigateToMxToBottomSheet(val link: String) : SpaceDirectoryViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewModel.kt index 9173e15dac..9d74849386 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/explore/SpaceDirectoryViewModel.kt @@ -40,7 +40,7 @@ import timber.log.Timber class SpaceDirectoryViewModel @AssistedInject constructor( @Assisted initialState: SpaceDirectoryState, - val session: Session + private val session: Session ) : VectorViewModel(initialState) { @AssistedFactory @@ -139,6 +139,13 @@ class SpaceDirectoryViewModel @AssistedInject constructor( is SpaceDirectoryViewAction.NavigateToRoom -> { _viewEvents.post(SpaceDirectoryViewEvents.NavigateToRoom(action.roomId)) } + is SpaceDirectoryViewAction.ShowDetails -> { + // This is temporary for now to at least display something for the space beta + // It's not ideal as it's doing some peeking that is not needed. + session.permalinkService().createRoomPermalink(action.spaceChildInfo.childRoomId)?.let { + _viewEvents.post(SpaceDirectoryViewEvents.NavigateToMxToBottomSheet(it)) + } + } } }