Improve room history visibility setting UX (#1579)

And observe correctly the state event
element-hq · Nov 26, 2020 · b679220 · b679220
1 parent 1f2c749
commit b679220
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 17 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -12,6 +12,7 @@ Improvements 🙌:
  - Handle events of type "m.room.server_acl" (#890)
  - Room creation form: add advanced section to disable federation (#1314)
  - Move "Enable Encryption" from room setting screen to room profile screen (#2394)
+ - Improve room history visibility setting UX (#1579)
 
 Bugfix 🐛:
  - Fix crash on AttachmentViewer (#2365)

diff --git a/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsController.kt b/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsController.kt
@@ -26,9 +26,6 @@ import im.vector.app.features.form.formEditTextItem
 import im.vector.app.features.form.formEditableAvatarItem
 import im.vector.app.features.home.AvatarRenderer
 import im.vector.app.features.home.room.detail.timeline.format.RoomHistoryVisibilityFormatter
-import org.matrix.android.sdk.api.session.events.model.Event
-import org.matrix.android.sdk.api.session.events.model.toModel
-import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibilityContent
 import org.matrix.android.sdk.api.util.toMatrixItem
 import javax.inject.Inject
 
@@ -60,9 +57,6 @@ class RoomSettingsController @Inject constructor(
     override fun buildModels(data: RoomSettingsViewState?) {
         val roomSummary = data?.roomSummary?.invoke() ?: return
 
-        val historyVisibility = data.historyVisibilityEvent?.let { formatRoomHistoryVisibilityEvent(it) } ?: ""
-        val newHistoryVisibility = data.newHistoryVisibility?.let { roomHistoryVisibilityFormatter.format(it) }
-
         formEditableAvatarItem {
             id("avatar")
             enabled(data.actionPermissions.canChangeAvatar)
@@ -119,6 +113,9 @@ class RoomSettingsController @Inject constructor(
             }
         }
 
+        val historyVisibility = roomHistoryVisibilityFormatter.format(data.currentHistoryVisibility)
+        val newHistoryVisibility = data.newHistoryVisibility?.let { roomHistoryVisibilityFormatter.format(it) }
+
         buildProfileAction(
                 id = "historyReadability",
                 title = stringProvider.getString(R.string.room_settings_room_read_history_rules_pref_title),
@@ -129,9 +126,4 @@ class RoomSettingsController @Inject constructor(
                 action = { if (data.actionPermissions.canChangeHistoryVisibility) callback?.onHistoryVisibilityClicked() }
         )
     }
-
-    private fun formatRoomHistoryVisibilityEvent(event: Event): String? {
-        val historyVisibility = event.getClearContent().toModel<RoomHistoryVisibilityContent>()?.historyVisibility ?: return null
-        return roomHistoryVisibilityFormatter.format(historyVisibility)
-    }
 }
diff --git a/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsFragment.kt b/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsFragment.kt
@@ -143,8 +143,7 @@ class RoomSettingsFragment @Inject constructor(
                 RoomHistoryVisibility.JOINED,
                 RoomHistoryVisibility.WORLD_READABLE
         )
-        val currentHistoryVisibility =
-                state.newHistoryVisibility ?: state.historyVisibilityEvent?.getClearContent().toModel<RoomHistoryVisibilityContent>()?.historyVisibility
+        val currentHistoryVisibility = state.newHistoryVisibility ?: state.currentHistoryVisibility
         val currentHistoryVisibilityIndex = historyVisibilities.indexOf(currentHistoryVisibility)
 
         AlertDialog.Builder(requireContext()).apply {

diff --git a/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewModel.kt b/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewModel.kt
@@ -33,6 +33,7 @@ import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.RoomAvatarContent
+import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibilityContent
 import org.matrix.android.sdk.api.session.room.powerlevels.PowerLevelsHelper
 import org.matrix.android.sdk.rx.mapOptional
 import org.matrix.android.sdk.rx.rx
@@ -60,6 +61,7 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
 
     init {
         observeRoomSummary()
+        observeRoomHistoryVisibility()
         observeRoomAvatar()
         observeState()
     }
@@ -84,7 +86,7 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
                                 || summary?.name != newName
                                 || summary?.topic != newTopic
                                 || summary?.canonicalAlias != newCanonicalAlias?.takeIf { it.isNotEmpty() }
-                                || newHistoryVisibility != null
+                                || (newHistoryVisibility != null && newHistoryVisibility != currentHistoryVisibility)
                 )
             }
         }
@@ -96,7 +98,6 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
                 .execute { async ->
                     val roomSummary = async.invoke()
                     copy(
-                            historyVisibilityEvent = room.getStateEvent(EventType.STATE_ROOM_HISTORY_VISIBILITY),
                             roomSummary = async,
                             newName = roomSummary?.name,
                             newTopic = roomSummary?.topic,
@@ -123,6 +124,19 @@ class RoomSettingsViewModel @AssistedInject constructor(@Assisted initialState:
                 .disposeOnClear()
     }
 
+    private fun observeRoomHistoryVisibility() {
+        room.rx()
+                .liveStateEvent(EventType.STATE_ROOM_HISTORY_VISIBILITY, QueryStringValue.NoCondition)
+                .mapOptional { it.content.toModel<RoomHistoryVisibilityContent>() }
+                .unwrap()
+                .subscribe {
+                    it.historyVisibility?.let {
+                        setState { copy(currentHistoryVisibility = it) }
+                    }
+                }
+                .disposeOnClear()
+    }
+
     /**
      * We do not want to use the fallback avatar url, which can be the other user avatar, or the current user avatar.
      */

diff --git a/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewState.kt b/vector/src/main/java/im/vector/app/features/roomprofile/settings/RoomSettingsViewState.kt
@@ -21,13 +21,13 @@ import com.airbnb.mvrx.Async
 import com.airbnb.mvrx.MvRxState
 import com.airbnb.mvrx.Uninitialized
 import im.vector.app.features.roomprofile.RoomProfileArgs
-import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility
 import org.matrix.android.sdk.api.session.room.model.RoomSummary
 
 data class RoomSettingsViewState(
         val roomId: String,
-        val historyVisibilityEvent: Event? = null,
+        // Default value: https://matrix.org/docs/spec/client_server/r0.6.1#id88
+        val currentHistoryVisibility: RoomHistoryVisibility = RoomHistoryVisibility.SHARED,
         val roomSummary: Async<RoomSummary> = Uninitialized,
         val isLoading: Boolean = false,
         val currentRoomAvatarUrl: String? = null,