Code review

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<SpaceChildContent>()
+                                            childStateEv.content.toModel<SpaceChildContent>()?.let { childStateEvContent ->
-                                            SpaceChildInfo(
+                                                SpaceChildInfo(
-                                                    childRoomId = childSummary.roomId,
+                                                        childRoomId = childSummary.roomId,
-                                                    isKnown = true,
+                                                        isKnown = true,
-                                                    roomType = childSummary.roomType,
+                                                        roomType = childSummary.roomType,
-                                                    name = childSummary.name,
+                                                        name = childSummary.name,
-                                                    topic = childSummary.topic,
+                                                        topic = childSummary.topic,
-                                                    avatarUrl = childSummary.avatarUrl,
+                                                        avatarUrl = childSummary.avatarUrl,
-                                                    order = childStateEvContent?.order,
+                                                        order = childStateEvContent?.order,
-                                                    autoJoin = childStateEvContent?.autoJoin ?: false,
+                                                        autoJoin = childStateEvContent?.autoJoin ?: false,
-                                                    viaServers = childStateEvContent?.via ?: emptyList(),
+                                                        viaServers = childStateEvContent?.via ?: emptyList(),
-                                                    activeMemberCount = childSummary.numJoinedMembers,
+                                                        activeMemberCount = childSummary.numJoinedMembers,
-                                                    parentRoomId = childStateEv.roomId
+                                                        parentRoomId = childStateEv.roomId
-                                            )
+                                                )
+                                            }
                                         }.orEmpty()
                             }
                             .orEmpty()

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<InteractionListener>(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)

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 ->
+                if (someYouKnow.isEmpty()) {
-                    images[index].isVisible = true
+                    views.peopleYouMayKnowText.isVisible = false
-                    avatarRenderer.render(item, images[index])
+                } 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

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<ActivitySimpleBinding>(), SpaceDirectoryViewModel.Factory {
+class SpaceExploreActivity : VectorBaseActivity<ActivitySimpleBinding>(), SpaceDirectoryViewModel.Factory, MatrixToBottomSheet.InteractionListener {
 
     @Inject lateinit var spaceDirectoryViewModelFactory: SpaceDirectoryViewModel.Factory
 
@@ -72,10 +74,20 @@ class SpaceExploreActivity : VectorBaseActivity<ActivitySimpleBinding>(), 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<ActivitySimpleBinding>(), SpaceD
 
     override fun create(initialState: SpaceDirectoryState): SpaceDirectoryViewModel =
             spaceDirectoryViewModelFactory.create(initialState)
+
+    override fun navigateToRoom(roomId: String) {
+        navigator.openRoom(this, roomId)
+    }
 }

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<FragmentRoomDirectoryPickerBinding>(),
         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
+        viewModel.handle(SpaceDirectoryViewAction.ShowDetails(spaceChildInfo))
-        // 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")
-        }
     }
 
     override fun onBackPressed(toolbarButton: Boolean): Boolean {
@@ -109,7 +94,7 @@ class SpaceDirectoryFragment @Inject constructor(
         return true
     }
 
-    override fun navigateToRoom(roomId: String) {
+//    override fun navigateToRoom(roomId: String) {
-        viewModel.handle(SpaceDirectoryViewAction.NavigateToRoom(roomId))
+//        viewModel.handle(SpaceDirectoryViewAction.NavigateToRoom(roomId))
-    }
+//    }
 }

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()
 }

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()
 }

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<SpaceDirectoryState, SpaceDirectoryViewAction, SpaceDirectoryViewEvents>(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))
+                }
+            }
         }
     }