Improve form UX + local echo in case of success

2025-03-17 19:58:57 +03:00 · 2020-11-24 06:04:44 +01:00 · 2020-11-24 06:04:44 +01:00 · 3a06ef3959
commit 3a06ef3959
parent ed4676bb6c
7 changed files with 149 additions and 44 deletions
--- a/vector/src/main/java/im/vector/app/features/discovery/SettingsContinueCancelItem.kt
+++ b/vector/src/main/java/im/vector/app/features/discovery/SettingsContinueCancelItem.kt
@ -27,6 +27,9 @@ import im.vector.app.core.epoxy.onClick
@EpoxyModelClass(layout = R.layout.item_settings_continue_cancel)
 abstract class SettingsContinueCancelItem : EpoxyModelWithHolder<SettingsContinueCancelItem.Holder>() {

+    @EpoxyAttribute
+    var continueText: String? = null
+
    @EpoxyAttribute
    var continueOnClick: ClickListener? = null

@ -37,6 +40,8 @@ abstract class SettingsContinueCancelItem : EpoxyModelWithHolder<SettingsContinu
        super.bind(holder)

        holder.cancelButton.onClick(cancelOnClick)
+
+        continueText?.let { holder.continueButton.text = it }
        holder.continueButton.onClick(continueOnClick)
    }

--- a/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasAction.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasAction.kt
@ -20,6 +20,7 @@ import im.vector.app.core.platform.VectorViewModelAction

 sealed class RoomAliasAction : VectorViewModelAction {
    // Canonical
+    object ToggleManualPublishForm : RoomAliasAction()
    data class SetNewAlias(val aliasLocalPart: String) : RoomAliasAction()
    object AddAlias : RoomAliasAction()
    data class RemoveAlias(val alias: String) : RoomAliasAction()
@ -27,6 +28,7 @@ sealed class RoomAliasAction : VectorViewModelAction {

    // Local
    data class RemoveLocalAlias(val alias: String) : RoomAliasAction()
+    object ToggleAddLocalAliasForm : RoomAliasAction()
    data class SetNewLocalAliasLocalPart(val aliasLocalPart: String) : RoomAliasAction()
    object AddLocalAlias : RoomAliasAction()
 }
--- a/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasController.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasController.kt
@ -25,10 +25,12 @@ import im.vector.app.core.epoxy.errorWithRetryItem
 import im.vector.app.core.epoxy.loadingItem
 import im.vector.app.core.epoxy.profiles.buildProfileSection
 import im.vector.app.core.error.ErrorFormatter
+import im.vector.app.core.resources.ColorProvider
 import im.vector.app.core.resources.StringProvider
+import im.vector.app.features.discovery.settingsButtonItem
+import im.vector.app.features.discovery.settingsContinueCancelItem
 import im.vector.app.features.discovery.settingsInfoItem
 import im.vector.app.features.form.formEditTextItem
-import im.vector.app.features.form.formSubmitButtonItem
 import im.vector.app.features.roomdirectory.createroom.RoomAliasErrorFormatter
 import im.vector.app.features.roomdirectory.createroom.roomAliasEditItem
 import im.vector.app.features.settings.threepids.threePidItem
@ -38,15 +40,18 @@ import javax.inject.Inject
 class RoomAliasController @Inject constructor(
        private val stringProvider: StringProvider,
        private val errorFormatter: ErrorFormatter,
+        private val colorProvider: ColorProvider,
        private val roomAliasErrorFormatter: RoomAliasErrorFormatter
 ) : TypedEpoxyController<RoomAliasViewState>() {

    interface Callback {
+        fun toggleManualPublishForm()
        fun setNewAlias(value: String)
        fun addAlias()
        fun removeAlias(altAlias: String)
        fun setCanonicalAlias(alias: String?)
        fun removeLocalAlias(alias: String)
+        fun toggleLocalAliasForm()
        fun setNewLocalAliasLocalPart(value: String)
        fun addLocalAlias()
    }
@ -104,19 +109,37 @@ class RoomAliasController @Inject constructor(
        }

        if (data.actionPermissions.canChangeCanonicalAlias) {
-            formEditTextItem {
-                id("addAlias")
-                value(data.newAlias)
-                showBottomSeparator(false)
-                hint(stringProvider.getString(R.string.room_alias_address_hint))
-                onTextChange { text ->
-                    callback?.setNewAlias(text)
+            buildPublishManuallyForm(data)
+        }
+    }
+
+    private fun buildPublishManuallyForm(data: RoomAliasViewState) {
+        when (data.publishManuallyState) {
+            RoomAliasViewState.AddAliasState.Hidden -> Unit
+            RoomAliasViewState.AddAliasState.Closed -> {
+                settingsButtonItem {
+                    id("publishManually")
+                    colorProvider(colorProvider)
+                    buttonTitleId(R.string.room_alias_published_alias_add_manually)
+                    buttonClickListener { callback?.toggleManualPublishForm() }
                }
            }
-            formSubmitButtonItem {
-                id("submit")
-                buttonTitleId(R.string.action_add)
-                buttonClickListener { callback?.addAlias() }
+            is RoomAliasViewState.AddAliasState.Editing -> {
+                formEditTextItem {
+                    id("publishManuallyEdit")
+                    value(data.publishManuallyState.value)
+                    showBottomSeparator(false)
+                    hint(stringProvider.getString(R.string.room_alias_address_hint))
+                    onTextChange { text ->
+                        callback?.setNewAlias(text)
+                    }
+                }
+                settingsContinueCancelItem {
+                    id("publishManuallySubmit")
+                    continueText(stringProvider.getString(R.string.room_alias_published_alias_add_manually_submit))
+                    continueOnClick { callback?.addAlias() }
+                    cancelOnClick { callback?.toggleManualPublishForm() }
+                }
            }
        }
    }
@ -155,21 +178,38 @@ class RoomAliasController @Inject constructor(
        }

        // Add local
-        roomAliasEditItem {
-            id("newLocalAlias")
-            value(data.newLocalAlias)
-            homeServer(":" + data.homeServerName)
-            showBottomSeparator(false)
-            errorMessage(roomAliasErrorFormatter.format((data.asyncNewLocalAliasRequest as? Fail)?.error as? RoomAliasError))
-            onTextChange { value ->
-                callback?.setNewLocalAliasLocalPart(value)
-            }
-        }
+        buildAddLocalAlias(data)
+    }

-        formSubmitButtonItem {
-            id("submitLocal")
-            buttonTitleId(R.string.action_add)
-            buttonClickListener { callback?.addLocalAlias() }
+    private fun buildAddLocalAlias(data: RoomAliasViewState) {
+        when (data.newLocalAliasState) {
+            RoomAliasViewState.AddAliasState.Hidden -> Unit
+            RoomAliasViewState.AddAliasState.Closed -> {
+                settingsButtonItem {
+                    id("newLocalAliasButton")
+                    colorProvider(colorProvider)
+                    buttonTitleId(R.string.room_alias_local_address_add)
+                    buttonClickListener { callback?.toggleLocalAliasForm() }
+                }
+            }
+            is RoomAliasViewState.AddAliasState.Editing -> {
+                roomAliasEditItem {
+                    id("newLocalAlias")
+                    value(data.newLocalAliasState.value)
+                    homeServer(":" + data.homeServerName)
+                    showBottomSeparator(false)
+                    errorMessage(roomAliasErrorFormatter.format((data.newLocalAliasState.asyncRequest as? Fail)?.error as? RoomAliasError))
+                    onTextChange { value ->
+                        callback?.setNewLocalAliasLocalPart(value)
+                    }
+                }
+                settingsContinueCancelItem {
+                    id("newLocalAliasSubmit")
+                    continueText(stringProvider.getString(R.string.action_add))
+                    continueOnClick { callback?.addLocalAlias() }
+                    cancelOnClick { callback?.toggleLocalAliasForm() }
+                }
+            }
        }
    }
 }
--- a/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasFragment.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasFragment.kt
@ -116,6 +116,10 @@ class RoomAliasFragment @Inject constructor(
        viewModel.handle(RoomAliasAction.SetCanonicalAlias(alias))
    }

+    override fun toggleManualPublishForm() {
+        viewModel.handle(RoomAliasAction.ToggleManualPublishForm)
+    }
+
    override fun setNewAlias(value: String) {
        viewModel.handle(RoomAliasAction.SetNewAlias(value))
    }
@ -124,6 +128,10 @@ class RoomAliasFragment @Inject constructor(
        viewModel.handle(RoomAliasAction.AddAlias)
    }

+    override fun toggleLocalAliasForm() {
+        viewModel.handle(RoomAliasAction.ToggleAddLocalAliasForm)
+    }
+
    override fun setNewLocalAliasLocalPart(value: String) {
        viewModel.handle(RoomAliasAction.SetNewLocalAliasLocalPart(value))
    }
--- a/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasViewModel.kt
@ -142,20 +142,46 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo

    override fun handle(action: RoomAliasAction) {
        when (action) {
+            RoomAliasAction.ToggleManualPublishForm      -> handleToggleManualPublishForm()
            is RoomAliasAction.SetNewAlias               -> handleSetNewAlias(action)
            is RoomAliasAction.AddAlias                  -> handleAddAlias()
            is RoomAliasAction.RemoveAlias               -> handleRemoveAlias(action)
            is RoomAliasAction.SetCanonicalAlias         -> handleSetCanonicalAlias(action)
+            RoomAliasAction.ToggleAddLocalAliasForm      -> handleToggleAddLocalAliasForm()
            is RoomAliasAction.SetNewLocalAliasLocalPart -> handleSetNewLocalAliasLocalPart(action)
            RoomAliasAction.AddLocalAlias                -> handleAddLocalAlias()
            is RoomAliasAction.RemoveLocalAlias          -> handleRemoveLocalAlias(action)
        }.exhaustive
    }

+    private fun handleToggleAddLocalAliasForm() {
+        setState {
+            copy(
+                    newLocalAliasState = when (newLocalAliasState) {
+                        RoomAliasViewState.AddAliasState.Hidden     -> RoomAliasViewState.AddAliasState.Hidden
+                        RoomAliasViewState.AddAliasState.Closed     -> RoomAliasViewState.AddAliasState.Editing("", Uninitialized)
+                        is RoomAliasViewState.AddAliasState.Editing -> RoomAliasViewState.AddAliasState.Closed
+                    }
+            )
+        }
+    }
+
+    private fun handleToggleManualPublishForm() {
+        setState {
+            copy(
+                    publishManuallyState = when (publishManuallyState) {
+                        RoomAliasViewState.AddAliasState.Hidden     -> RoomAliasViewState.AddAliasState.Hidden
+                        RoomAliasViewState.AddAliasState.Closed     -> RoomAliasViewState.AddAliasState.Editing("", Uninitialized)
+                        is RoomAliasViewState.AddAliasState.Editing -> RoomAliasViewState.AddAliasState.Closed
+                    }
+            )
+        }
+    }
+
    private fun handleSetNewAlias(action: RoomAliasAction.SetNewAlias) {
        setState {
            copy(
-                    newAlias = action.aliasLocalPart
+                    publishManuallyState = RoomAliasViewState.AddAliasState.Editing(action.aliasLocalPart, Uninitialized)
            )
        }
    }
@ -163,16 +189,16 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo
    private fun handleSetNewLocalAliasLocalPart(action: RoomAliasAction.SetNewLocalAliasLocalPart) {
        setState {
            copy(
-                    newLocalAlias = action.aliasLocalPart,
-                    asyncNewLocalAliasRequest = Uninitialized
+                    newLocalAliasState = RoomAliasViewState.AddAliasState.Editing(action.aliasLocalPart, Uninitialized)
            )
        }
    }

    private fun handleAddAlias() = withState { state ->
+        val newAlias = (state.newLocalAliasState as? RoomAliasViewState.AddAliasState.Editing)?.value ?: return@withState
        updateCanonicalAlias(
                state.canonicalAlias,
-                state.alternativeAliases + state.newAlias
+                state.alternativeAliases + newAlias
        )
    }

@ -197,7 +223,7 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo
                setState {
                    copy(
                            isLoading = false,
-                            newAlias = ""
+                            publishManuallyState = RoomAliasViewState.AddAliasState.Closed
                    )
                }
            }
@ -206,24 +232,25 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo
                postLoading(false)
                _viewEvents.post(RoomAliasViewEvents.Failure(failure))
            }
-        }
-        )
+        })
    }

    private fun handleAddLocalAlias() = withState { state ->
+        val previousState = (state.newLocalAliasState as? RoomAliasViewState.AddAliasState.Editing) ?: return@withState
+
        setState {
            copy(
                    isLoading = true,
-                    asyncNewLocalAliasRequest = Loading()
+                    newLocalAliasState = previousState.copy(asyncRequest = Loading())
            )
        }
        viewModelScope.launch {
-            runCatching { room.addAlias(state.newLocalAlias) }
+            runCatching { room.addAlias(previousState.value) }
                    .onFailure {
                        setState {
                            copy(
                                    isLoading = false,
-                                    asyncNewLocalAliasRequest = Fail(it)
+                                    newLocalAliasState = previousState.copy(asyncRequest = Fail(it))
                            )
                        }
                        _viewEvents.post(RoomAliasViewEvents.Failure(it))
@ -232,8 +259,9 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo
                        setState {
                            copy(
                                    isLoading = false,
-                                    newLocalAlias = "",
-                                    asyncNewLocalAliasRequest = Uninitialized
+                                    newLocalAliasState = RoomAliasViewState.AddAliasState.Closed,
+                                    // Local echo
+                                    localAliases = Success(localAliases().orEmpty() + previousState.value)
                            )
                        }
                        fetchRoomAlias()
@ -245,9 +273,23 @@ class RoomAliasViewModel @AssistedInject constructor(@Assisted initialState: Roo
        postLoading(true)
        viewModelScope.launch {
            runCatching { session.deleteRoomAlias(action.alias) }
-                    .onFailure { _viewEvents.post(RoomAliasViewEvents.Failure(it)) }
-                    .onSuccess { fetchRoomAlias() }
-            postLoading(false)
+                    .onFailure {
+                        setState {
+                            copy(isLoading = false)
+                        }
+                        _viewEvents.post(RoomAliasViewEvents.Failure(it))
+                    }
+                    .onSuccess {
+                        // Local echo
+                        setState {
+                            copy(
+                                    isLoading = false,
+                                    // Local echo
+                                    localAliases = Success(localAliases().orEmpty() - action.alias)
+                            )
+                        }
+                        fetchRoomAlias()
+                    }
        }
    }

--- a/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasViewState.kt
+++ b/vector/src/main/java/im/vector/app/features/roomprofile/alias/RoomAliasViewState.kt
@ -30,10 +30,9 @@ data class RoomAliasViewState(
        val isLoading: Boolean = false,
        val canonicalAlias: String? = null,
        val alternativeAliases: List<String> = emptyList(),
-        val newAlias: String = "",
+        val publishManuallyState: AddAliasState = AddAliasState.Hidden,
        val localAliases: Async<List<String>> = Uninitialized,
-        val newLocalAlias: String = "",
-        val asyncNewLocalAliasRequest: Async<Unit> = Uninitialized
+        val newLocalAliasState: AddAliasState = AddAliasState.Hidden
 ) : MvRxState {

    constructor(args: RoomProfileArgs) : this(roomId = args.roomId)
@ -41,4 +40,10 @@ data class RoomAliasViewState(
    data class ActionPermissions(
            val canChangeCanonicalAlias: Boolean = false
    )
+
+    sealed class AddAliasState {
+        object Hidden : AddAliasState()
+        object Closed : AddAliasState()
+        data class Editing(val value: String, val asyncRequest: Async<Unit> = Uninitialized) : AddAliasState()
+    }
 }
--- a/vector/src/main/res/values/strings.xml
+++ b/vector/src/main/res/values/strings.xml
@ -1032,6 +1032,8 @@
    <string name="room_alias_published_alias_subtitle">Published addresses can be used by anyone on any server to join your room. To publish an address, it needs to be set as a local address first.</string>
    <string name="room_alias_main_address_hint">Main address</string>
    <string name="room_alias_published_other">Other published addresses:</string>
+    <string name="room_alias_published_alias_add_manually">Published a new address manually</string>
+    <string name="room_alias_published_alias_add_manually_submit">Publish</string>
    <string name="room_alias_delete_confirmation">Delete the address \"%1$s\"?</string>
    <!-- Parameter will be the url of the homeserver, ex: matrix.org -->
    <string name="room_alias_publish">Publish this room to the public in %1$s\'s room directory?</string>
@ -1043,6 +1045,7 @@
    <!-- Parameter will be the url of the homeserver, ex: matrix.org -->
    <string name="room_alias_local_address_subtitle">Set addresses for this room so users can find this room through your homeserver (%1$s)</string>
    <string name="room_alias_local_address_empty">This room has no local addresses</string>
+    <string name="room_alias_local_address_add">Add a local address</string>

    <!-- Room settings, access and visibility : WHO CAN READ HISTORY? (read rule) -->
    <string name="room_settings_read_history_entry_anyone">Anyone</string>