Code review

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/RoomSummaryMapper.kt

@@ -90,7 +90,6 @@ internal class RoomSummaryMapper @Inject constructor(private val timelineEventMa
                             autoJoin = it.autoJoin ?: false,
                             viaServers = it.viaServers.toList(),
                             parentRoomId = roomSummaryEntity.roomId,
-                            // What to do here?
                             suggested = it.suggested
                     )
                 },

vector/src/main/java/im/vector/app/core/extensions/EditText.kt

@@ -23,7 +23,9 @@ import android.view.View
 import android.view.inputmethod.EditorInfo
 import android.widget.EditText
 import androidx.annotation.DrawableRes
+import com.google.android.material.textfield.TextInputEditText
 import im.vector.app.R
+import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.platform.SimpleTextWatcher
 
 fun EditText.setupAsSearch(@DrawableRes searchIconRes: Int = R.drawable.ic_search,
@@ -57,3 +59,17 @@ fun EditText.setupAsSearch(@DrawableRes searchIconRes: Int = R.drawable.ic_searc
         return@OnTouchListener false
     })
 }
+
+/**
+ * Set the initial value of the textEdit.
+ * Avoids issue with two way bindings, the value is only set the first time
+ */
+fun TextInputEditText.setValueOnce(value: String?, holder: VectorEpoxyHolder) {
+    if (holder.view.isAttachedToWindow) {
+        // the view is attached to the window
+        // So it is a rebind of new data and you could ignore it assuming this is text that was already inputted into the view.
+        // Downside is if you ever wanted to programmatically change the content of the edit text while it is on screen you would not be able to
+    } else {
+        setText(value)
+    }
+}

vector/src/main/java/im/vector/app/features/form/FormEditTextItem.kt

@@ -27,6 +27,7 @@ import com.google.android.material.textfield.TextInputLayout
 import im.vector.app.R
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.extensions.setValueOnce
 import im.vector.app.core.platform.SimpleTextWatcher
 
 @EpoxyModelClass(layout = R.layout.item_form_text_input)
@@ -75,13 +76,8 @@ abstract class FormEditTextItem : VectorEpoxyModel<FormEditTextItem.Holder>() {
         holder.textInputLayout.error = errorMessage
         holder.textInputLayout.endIconMode = endIconMode ?: TextInputLayout.END_ICON_NONE
 
-        if (holder.view.isAttachedToWindow) {
-            // the view is attached to the window
-            // So it is a rebind of new data and you could ignore it assuming this is text that was already inputted into the view.
-            // Downside is if you ever wanted to programmatically change the content of the edit text while it is on screen you would not be able to
-        } else {
-            holder.textInputEditText.setText(value)
-        }
+        holder.textInputEditText.setValueOnce(value, holder)
+
         holder.textInputEditText.isEnabled = enabled
         inputType?.let { holder.textInputEditText.inputType = it }
         holder.textInputEditText.isSingleLine = singleLine ?: false

vector/src/main/java/im/vector/app/features/form/FormEditTextWithButtonItem.kt

@@ -26,6 +26,7 @@ import com.google.android.material.textfield.TextInputLayout
 import im.vector.app.R
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.extensions.setValueOnce
 import im.vector.app.core.platform.SimpleTextWatcher
 
 @EpoxyModelClass(layout = R.layout.item_form_text_input_with_button)
@@ -60,12 +61,8 @@ abstract class FormEditTextWithButtonItem : VectorEpoxyModel<FormEditTextWithBut
         holder.textInputLayout.isEnabled = enabled
         holder.textInputLayout.hint = hint
 
-        if (holder.view.isAttachedToWindow) {
-            // the view is attached to the window
-            // So it is a rebind of new data and you could ignore it assuming this is text that was already inputted into the view.
-        } else {
-            holder.textInputEditText.setText(value)
-        }
+        holder.textInputEditText.setValueOnce(value, holder)
+
         holder.textInputEditText.isEnabled = enabled
 
         holder.textInputEditText.addTextChangedListener(onTextChangeListener)

vector/src/main/java/im/vector/app/features/form/FormMultiLineEditTextItem.kt

@@ -27,6 +27,7 @@ import com.google.android.material.textfield.TextInputLayout
 import im.vector.app.R
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.extensions.setValueOnce
 import im.vector.app.core.platform.SimpleTextWatcher
 
 @EpoxyModelClass(layout = R.layout.item_form_multiline_text_input)
@@ -75,13 +76,8 @@ abstract class FormMultiLineEditTextItem : VectorEpoxyModel<FormMultiLineEditTex
         holder.textInputEditText.textSize = textSizeSp?.toFloat() ?: 14f
         holder.textInputEditText.minLines = minLines
 
-        // Update only if text is different and value is not null
-        if (holder.view.isAttachedToWindow) {
-            // the view is attached to the window
-            // So it is a rebind of new data and you could ignore it assuming this is text that was already inputted into the view.
-        } else {
-            holder.textInputEditText.setText(value)
-        }
+        holder.textInputEditText.setValueOnce(value, holder)
+
         holder.textInputEditText.isEnabled = enabled
 
         holder.textInputEditText.addTextChangedListener(onTextChangeListener)

vector/src/main/java/im/vector/app/features/roomdirectory/createroom/RoomAliasEditItem.kt

@@ -27,6 +27,7 @@ import com.google.android.material.textfield.TextInputLayout
 import im.vector.app.R
 import im.vector.app.core.epoxy.VectorEpoxyHolder
 import im.vector.app.core.epoxy.VectorEpoxyModel
+import im.vector.app.core.extensions.setValueOnce
 import im.vector.app.core.platform.SimpleTextWatcher
 
 @EpoxyModelClass(layout = R.layout.item_room_alias_text_input)
@@ -61,12 +62,7 @@ abstract class RoomAliasEditItem : VectorEpoxyModel<RoomAliasEditItem.Holder>()
         holder.textInputLayout.isEnabled = enabled
         holder.textInputLayout.error = errorMessage
 
-        if (holder.view.isAttachedToWindow) {
-            // the view is attached to the window
-            // So it is a rebind of new data and you could ignore it assuming this is text that was already inputted into the view.
-        } else {
-            holder.textInputEditText.setText(value)
-        }
+        holder.textInputEditText.setValueOnce(value, holder)
         holder.textInputEditText.isEnabled = enabled
         holder.textInputEditText.addTextChangedListener(onTextChangeListener)
         holder.homeServerText.text = homeServer

vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsController.kt

@@ -27,7 +27,6 @@ import im.vector.app.features.form.formEditableAvatarItem
 import im.vector.app.features.form.formSwitchItem
 import im.vector.app.features.home.AvatarRenderer
 import im.vector.app.features.home.room.detail.timeline.format.RoomHistoryVisibilityFormatter
-import im.vector.app.features.roomprofile.settings.RoomSettingsViewState.Companion.getJoinRuleWording
 import im.vector.app.features.settings.VectorPreferences
 import org.matrix.android.sdk.api.session.room.model.GuestAccess
 import org.matrix.android.sdk.api.session.room.model.RoomJoinRules

vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewState.kt

@@ -70,26 +70,24 @@ data class RoomSettingsViewState(
     ) {
         fun hasChanged() = newJoinRules != null || newGuestAccess != null
     }
+}
 
-    companion object {
-        fun RoomSettingsViewState.getJoinRuleWording(stringProvider: StringProvider): String {
-            return when (val joinRule = newRoomJoinRules.newJoinRules ?: currentRoomJoinRules) {
-                RoomJoinRules.INVITE -> {
-                    stringProvider.getString(R.string.room_settings_room_access_private_title)
-                }
-                RoomJoinRules.PUBLIC -> {
-                    stringProvider.getString(R.string.room_settings_room_access_public_title)
-                }
-                RoomJoinRules.KNOCK -> {
-                    stringProvider.getString(R.string.room_settings_room_access_entry_knock)
-                }
-                RoomJoinRules.RESTRICTED -> {
-                    stringProvider.getString(R.string.room_settings_room_access_restricted_title)
-                }
-                else                     -> {
-                    stringProvider.getString(R.string.room_settings_room_access_entry_unknown, joinRule.value)
-                }
-            }
+fun RoomSettingsViewState.getJoinRuleWording(stringProvider: StringProvider): String {
+    return when (val joinRule = newRoomJoinRules.newJoinRules ?: currentRoomJoinRules) {
+        RoomJoinRules.INVITE -> {
+            stringProvider.getString(R.string.room_settings_room_access_private_title)
+        }
+        RoomJoinRules.PUBLIC -> {
+            stringProvider.getString(R.string.room_settings_room_access_public_title)
+        }
+        RoomJoinRules.KNOCK -> {
+            stringProvider.getString(R.string.room_settings_room_access_entry_knock)
+        }
+        RoomJoinRules.RESTRICTED -> {
+            stringProvider.getString(R.string.room_settings_room_access_restricted_title)
+        }
+        else                     -> {
+            stringProvider.getString(R.string.room_settings_room_access_entry_unknown, joinRule.value)
         }
     }
 }

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceChildInfoMatchFilter.kt

+/*
+ * Copyright (c) 2021 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.spaces.manage
+
+import io.reactivex.functions.Predicate
+import org.matrix.android.sdk.api.extensions.orFalse
+import org.matrix.android.sdk.api.session.room.model.SpaceChildInfo
+
+class SpaceChildInfoMatchFilter : Predicate<SpaceChildInfo> {
+    var filter: String = ""
+
+    override fun test(spaceChildInfo: SpaceChildInfo): Boolean {
+        if (filter.isEmpty()) {
+            // No filter
+            return true
+        }
+        // if filter is "Jo Do", it should match "John Doe"
+        return filter.split(" ").all {
+            spaceChildInfo.name?.contains(it, ignoreCase = true).orFalse()
+                    || spaceChildInfo.topic?.contains(it, ignoreCase = true).orFalse()
+        }
+    }
+}

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageActivity.kt

@@ -111,17 +111,19 @@ class SpaceManageActivity : VectorBaseActivity<ActivitySimpleLoadingBinding>(),
                     }
                     ManageType.Settings -> {
                         val simpleName = SpaceSettingsFragment::class.java.simpleName
-                        if (supportFragmentManager.findFragmentByTag(simpleName) == null) {
+                        if (supportFragmentManager.findFragmentByTag(simpleName) == null && args?.spaceId != null) {
                             supportFragmentManager.commitTransaction {
                                 replace(R.id.simpleFragmentContainer,
                                         SpaceSettingsFragment::class.java,
-                                        Bundle().apply { this.putParcelable(MvRx.KEY_ARG, RoomProfileArgs(args?.spaceId ?: "")) },
+                                        Bundle().apply { this.putParcelable(MvRx.KEY_ARG, RoomProfileArgs(args.spaceId)) },
                                         simpleName
                                 )
                             }
                         }
                     }
-                    ManageType.ManageRooms -> TODO()
+                    ManageType.ManageRooms -> {
+                        // no direct access for now
+                    }
                 }
             }
         }
@@ -145,11 +147,13 @@ class SpaceManageActivity : VectorBaseActivity<ActivitySimpleLoadingBinding>(),
                     )
                 }
                 SpaceManagedSharedViewEvents.NavigateToManageRooms -> {
-                    addFragmentToBackstack(
-                            R.id.simpleFragmentContainer,
-                            SpaceManageRoomsFragment::class.java,
-                            SpaceManageArgs(args?.spaceId ?: "", ManageType.ManageRooms)
-                    )
+                    args?.spaceId?.let { spaceId ->
+                        addFragmentToBackstack(
+                                R.id.simpleFragmentContainer,
+                                SpaceManageRoomsFragment::class.java,
+                                SpaceManageArgs(spaceId, ManageType.ManageRooms)
+                        )
+                    }
                 }
             }
         }

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageRoomViewEvents.kt

@@ -19,6 +19,6 @@ package im.vector.app.features.spaces.manage
 import im.vector.app.core.platform.VectorViewEvents
 
 sealed class SpaceManageRoomViewEvents : VectorViewEvents {
-    object BulkActionSuccess: SpaceManageRoomViewEvents()
-    data class BulkActionFailure(val errorList: List<String>) : SpaceManageRoomViewEvents()
+//    object BulkActionSuccess: SpaceManageRoomViewEvents()
+    data class BulkActionFailure(val errorList: List<Throwable>) : SpaceManageRoomViewEvents()
 }

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageRoomsController.kt

@@ -24,8 +24,6 @@ import im.vector.app.core.epoxy.loadingItem
 import im.vector.app.core.error.ErrorFormatter
 import im.vector.app.core.utils.DebouncedClickListener
 import im.vector.app.features.home.AvatarRenderer
-import io.reactivex.functions.Predicate
-import org.matrix.android.sdk.api.extensions.orFalse
 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
@@ -83,19 +81,3 @@ class SpaceManageRoomsController @Inject constructor(
         }
     }
 }
-
-class SpaceChildInfoMatchFilter : Predicate<SpaceChildInfo> {
-    var filter: String = ""
-
-    override fun test(spaceChildInfo: SpaceChildInfo): Boolean {
-        if (filter.isEmpty()) {
-            // No filter
-            return true
-        }
-        // if filter is "Jo Do", it should match "John Doe"
-        return filter.split(" ").all {
-            spaceChildInfo.name?.contains(it, ignoreCase = true).orFalse()
-                    || spaceChildInfo.topic?.contains(it, ignoreCase = true).orFalse()
-        }
-    }
-}

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageRoomsFragment.kt

@@ -91,12 +91,7 @@ class SpaceManageRoomsFragment @Inject constructor(
         viewModel.observeViewEvents {
             when (it) {
                 is SpaceManageRoomViewEvents.BulkActionFailure -> {
-                    it.errorList.forEach {
-                        vectorBaseActivity.toast(it)
-                    }
-                }
-                SpaceManageRoomViewEvents.BulkActionSuccess -> {
-                    //
+                    vectorBaseActivity.toast(errorFormatter.toHumanReadable(it.errorList.firstOrNull()))
                 }
             }
         }

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageRoomsViewModel.kt

@@ -27,7 +27,6 @@ import com.airbnb.mvrx.ViewModelContext
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import im.vector.app.core.error.ErrorFormatter
 import im.vector.app.core.mvrx.runCatchingToAsync
 import im.vector.app.core.platform.VectorViewModel
 import im.vector.app.features.session.coroutineScope
@@ -37,8 +36,7 @@ import org.matrix.android.sdk.api.session.Session
 
 class SpaceManageRoomsViewModel @AssistedInject constructor(
         @Assisted val initialState: SpaceManageRoomViewState,
-        private val session: Session,
-        private val errorFormatter: ErrorFormatter
+        private val session: Session
 ) : VectorViewModel<SpaceManageRoomViewState, SpaceManageRoomViewAction, SpaceManageRoomViewEvents>(initialState) {
 
     init {
@@ -79,20 +77,20 @@ class SpaceManageRoomsViewModel @AssistedInject constructor(
 
     override fun handle(action: SpaceManageRoomViewAction) {
         when (action) {
-            is SpaceManageRoomViewAction.ToggleSelection    -> handleToggleSelection(action)
-            is SpaceManageRoomViewAction.UpdateFilter       -> {
+            is SpaceManageRoomViewAction.ToggleSelection -> handleToggleSelection(action)
+            is SpaceManageRoomViewAction.UpdateFilter -> {
                 setState { copy(currentFilter = action.filter) }
             }
-            SpaceManageRoomViewAction.ClearSelection        -> {
+            SpaceManageRoomViewAction.ClearSelection -> {
                 setState { copy(selectedRooms = emptyList()) }
             }
-            SpaceManageRoomViewAction.BulkRemove            -> {
+            SpaceManageRoomViewAction.BulkRemove -> {
                 handleBulkRemove()
             }
             is SpaceManageRoomViewAction.MarkAllAsSuggested -> {
                 handleBulkMarkAsSuggested(action.suggested)
             }
-            SpaceManageRoomViewAction.RefreshFromServer     -> {
+            SpaceManageRoomViewAction.RefreshFromServer -> {
                 refreshSummaryAPI()
             }
         }
@@ -112,10 +110,10 @@ class SpaceManageRoomsViewModel @AssistedInject constructor(
             }
             if (errorList.isEmpty()) {
                 // success
-                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionSuccess)
             } else {
-                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionFailure(errorList.map { errorFormatter.toHumanReadable(it) }))
+                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionFailure(errorList))
             }
+            refreshSummaryAPI()
             setState { copy(actionState = Uninitialized) }
         }
     }
@@ -142,11 +140,10 @@ class SpaceManageRoomsViewModel @AssistedInject constructor(
             }
             if (errorList.isEmpty()) {
                 // success
-                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionSuccess)
-                refreshSummaryAPI()
             } else {
-                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionFailure(errorList.map { errorFormatter.toHumanReadable(it) }))
+                _viewEvents.post(SpaceManageRoomViewEvents.BulkActionFailure(errorList))
             }
+            refreshSummaryAPI()
             setState { copy(actionState = Uninitialized) }
         }
     }

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceSettingsController.kt

@@ -28,7 +28,7 @@ import im.vector.app.features.form.formMultiLineEditTextItem
 import im.vector.app.features.form.formSwitchItem
 import im.vector.app.features.home.AvatarRenderer
 import im.vector.app.features.roomprofile.settings.RoomSettingsViewState
-import im.vector.app.features.roomprofile.settings.RoomSettingsViewState.Companion.getJoinRuleWording
+import im.vector.app.features.roomprofile.settings.getJoinRuleWording
 import im.vector.app.features.settings.VectorPreferences
 import org.matrix.android.sdk.api.session.room.model.RoomJoinRules
 import org.matrix.android.sdk.api.util.toMatrixItem

vector/src/main/java/im/vector/app/features/spaces/manage/SpaceSettingsFragment.kt

@@ -195,7 +195,9 @@ class SpaceSettingsFragment @Inject constructor(
         viewModel.handle(RoomSettingsAction.SetRoomTopic(topic))
     }
 
-    override fun onHistoryVisibilityClicked() {}
+    override fun onHistoryVisibilityClicked() {
+        // N/A for space settings screen
+    }
 
     override fun onJoinRuleClicked() = withState(viewModel) { state ->
         val currentJoinRule = state.newRoomJoinRules.newJoinRules ?: state.currentRoomJoinRules

vector/src/main/res/values/styles_riot.xml

@@ -11,7 +11,7 @@
         <item name="android:background">?riotx_background</item>
     </style>
 
-    <style name="ActionModeTheme" parent="Base.Widget.AppCompat.ActionMode">
+    <style name="ActionModeTheme" parent="Widget.AppCompat.ActionMode">
         <item name="background">?riotx_background</item>
         <item name="titleTextStyle">@style/Vector.Toolbar.Title</item>
         <item name="subtitleTextStyle">@style/Vector.Toolbar.SubTitle</item>