From 6b09a78ecefe9cda054aa85a8319f19a6882c069 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 13 May 2020 23:33:51 +0200 Subject: [PATCH] Identity: Improve identity choice screen after review --- .../riotx/core/platform/VectorBaseFragment.kt | 14 ++ .../change/SetIdentityServerAction.kt | 5 +- .../change/SetIdentityServerFragment.kt | 121 ++++++++---------- .../change/SetIdentityServerState.kt | 8 +- .../change/SetIdentityServerViewEvents.kt | 8 +- .../change/SetIdentityServerViewModel.kt | 73 +++++------ .../layout/fragment_set_identity_server.xml | 76 +++++++---- vector/src/main/res/values/strings.xml | 8 +- 8 files changed, 165 insertions(+), 148 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/core/platform/VectorBaseFragment.kt b/vector/src/main/java/im/vector/riotx/core/platform/VectorBaseFragment.kt index 6eb316456a..c4dcb0d996 100644 --- a/vector/src/main/java/im/vector/riotx/core/platform/VectorBaseFragment.kt +++ b/vector/src/main/java/im/vector/riotx/core/platform/VectorBaseFragment.kt @@ -39,6 +39,7 @@ import com.airbnb.mvrx.BaseMvRxFragment import com.airbnb.mvrx.MvRx import com.bumptech.glide.util.Util.assertMainThread import com.google.android.material.snackbar.Snackbar +import com.jakewharton.rxbinding3.view.clicks import im.vector.riotx.R import im.vector.riotx.core.di.DaggerScreenComponent import im.vector.riotx.core.di.HasScreenInjector @@ -49,6 +50,7 @@ import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.CompositeDisposable import io.reactivex.disposables.Disposable import timber.log.Timber +import java.util.concurrent.TimeUnit abstract class VectorBaseFragment : BaseMvRxFragment(), HasScreenInjector { @@ -249,6 +251,18 @@ abstract class VectorBaseFragment : BaseMvRxFragment(), HasScreenInjector { .disposeOnDestroyView() } + /* ========================================================================================== + * Views + * ========================================================================================== */ + + protected fun View.debouncedClicks(onClicked: () -> Unit) { + clicks() + .throttleFirst(300, TimeUnit.MILLISECONDS) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { onClicked() } + .disposeOnDestroyView() + } + /* ========================================================================================== * MENU MANAGEMENT * ========================================================================================== */ diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerAction.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerAction.kt index 1f38de1ffb..14f149c282 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerAction.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerAction.kt @@ -19,6 +19,7 @@ package im.vector.riotx.features.discovery.change import im.vector.riotx.core.platform.VectorViewModelAction sealed class SetIdentityServerAction : VectorViewModelAction { - data class UpdateIdentityServerUrl(val url: String) : SetIdentityServerAction() - object DoChangeIdentityServerUrl : SetIdentityServerAction() + object UseDefaultIdentityServer : SetIdentityServerAction() + + data class UseCustomIdentityServer(val url: String) : SetIdentityServerAction() } diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerFragment.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerFragment.kt index b5c237f110..360ca74f7b 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerFragment.kt @@ -18,25 +18,22 @@ package im.vector.riotx.features.discovery.change import android.app.Activity import android.content.Intent import android.os.Bundle -import android.text.Editable -import android.view.MenuItem import android.view.View import android.view.inputmethod.EditorInfo -import android.widget.EditText -import android.widget.ProgressBar import androidx.appcompat.app.AlertDialog import androidx.core.view.isVisible -import butterknife.BindView -import butterknife.OnTextChanged import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState -import com.google.android.material.textfield.TextInputLayout +import com.jakewharton.rxbinding3.widget.textChanges import im.vector.matrix.android.api.session.terms.TermsService import im.vector.riotx.R +import im.vector.riotx.core.extensions.exhaustive +import im.vector.riotx.core.extensions.toReducedUrl import im.vector.riotx.core.platform.VectorBaseActivity import im.vector.riotx.core.platform.VectorBaseFragment import im.vector.riotx.features.discovery.DiscoverySharedViewModel import im.vector.riotx.features.terms.ReviewTermsActivity +import kotlinx.android.synthetic.main.fragment_set_identity_server.* import javax.inject.Inject class SetIdentityServerFragment @Inject constructor( @@ -45,47 +42,26 @@ class SetIdentityServerFragment @Inject constructor( override fun getLayoutResId() = R.layout.fragment_set_identity_server - override fun getMenuRes() = R.menu.menu_submit - - @BindView(R.id.discovery_identity_server_enter_til) - lateinit var mKeyInputLayout: TextInputLayout - - @BindView(R.id.discovery_identity_server_enter_edittext) - lateinit var mKeyTextEdit: EditText - - @BindView(R.id.discovery_identity_server_loading) - lateinit var mProgressBar: ProgressBar - private val viewModel by fragmentViewModel(SetIdentityServerViewModel::class) lateinit var sharedViewModel: DiscoverySharedViewModel override fun invalidate() = withState(viewModel) { state -> - if (state.isVerifyingServer) { - mKeyTextEdit.isEnabled = false - mProgressBar.isVisible = true + if (state.defaultIdentityServerUrl.isNullOrEmpty()) { + // No default + identityServerSetDefaultNotice.isVisible = false + identityServerSetDefaultSubmit.isVisible = false + identityServerSetDefaultAlternative.setText(R.string.identity_server_set_alternative_notice_no_default) } else { - mKeyTextEdit.isEnabled = true - mProgressBar.isVisible = false - } - val newText = state.newIdentityServerUrl ?: "" - if (newText != mKeyTextEdit.text.toString()) { - mKeyTextEdit.setText(newText) - } - mKeyInputLayout.error = state.errorMessageId?.let { getString(it) } - } - - override fun onOptionsItemSelected(item: MenuItem): Boolean { - return when (item.itemId) { - R.id.action_submit -> { - withState(viewModel) { state -> - if (!state.isVerifyingServer) { - viewModel.handle(SetIdentityServerAction.DoChangeIdentityServerUrl) - } - } - true - } - else -> super.onOptionsItemSelected(item) + identityServerSetDefaultNotice.text = getString( + R.string.identity_server_set_default_notice, + state.homeServerUrl.toReducedUrl(), + state.defaultIdentityServerUrl.toReducedUrl() + ) + identityServerSetDefaultNotice.isVisible = true + identityServerSetDefaultSubmit.isVisible = true + identityServerSetDefaultSubmit.text = getString(R.string.identity_server_set_default_submit, state.defaultIdentityServerUrl.toReducedUrl()) + identityServerSetDefaultAlternative.setText(R.string.identity_server_set_alternative_notice) } } @@ -94,20 +70,35 @@ class SetIdentityServerFragment @Inject constructor( sharedViewModel = activityViewModelProvider.get(DiscoverySharedViewModel::class.java) - mKeyTextEdit.setOnEditorActionListener { _, actionId, _ -> + identityServerSetDefaultAlternativeTextInput.setOnEditorActionListener { _, actionId, _ -> if (actionId == EditorInfo.IME_ACTION_DONE) { - withState(viewModel) { state -> - if (!state.isVerifyingServer) { - viewModel.handle(SetIdentityServerAction.DoChangeIdentityServerUrl) - } - } + viewModel.handle(SetIdentityServerAction.UseCustomIdentityServer(identityServerSetDefaultAlternativeTextInput.text.toString())) return@setOnEditorActionListener true } return@setOnEditorActionListener false } + identityServerSetDefaultAlternativeTextInput + .textChanges() + .subscribe { + identityServerSetDefaultAlternativeTil.error = null + identityServerSetDefaultAlternativeSubmit.isEnabled = it.isNotEmpty() + } + .disposeOnDestroyView() + + identityServerSetDefaultSubmit.debouncedClicks { + viewModel.handle(SetIdentityServerAction.UseDefaultIdentityServer) + } + + identityServerSetDefaultAlternativeSubmit.debouncedClicks { + viewModel.handle(SetIdentityServerAction.UseCustomIdentityServer(identityServerSetDefaultAlternativeTextInput.text.toString())) + } + viewModel.observeViewEvents { when (it) { + is SetIdentityServerViewEvents.Loading -> showLoading(it.message) + is SetIdentityServerViewEvents.Failure -> identityServerSetDefaultAlternativeTil.error = getString(it.errorMessageId) + is SetIdentityServerViewEvents.OtherFailure -> showFailure(it.failure) is SetIdentityServerViewEvents.NoTerms -> { AlertDialog.Builder(requireActivity()) .setTitle(R.string.settings_discovery_no_terms_title) @@ -117,23 +108,25 @@ class SetIdentityServerFragment @Inject constructor( } .setNegativeButton(R.string.cancel, null) .show() + Unit } - - is SetIdentityServerViewEvents.TermsAccepted -> { - processIdentityServerChange() - } - + is SetIdentityServerViewEvents.TermsAccepted -> processIdentityServerChange() is SetIdentityServerViewEvents.ShowTerms -> { navigator.openTerms( this, TermsService.ServiceType.IdentityService, - SetIdentityServerViewModel.sanitatizeBaseURL(it.newIdentityServer), + SetIdentityServerViewModel.sanitatizeBaseURL(it.identityServerUrl), null) } - } + }.exhaustive } } + override fun onResume() { + super.onResume() + (activity as? VectorBaseActivity)?.supportActionBar?.setTitle(R.string.identity_server) + } + override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { if (requestCode == ReviewTermsActivity.TERMS_REQUEST_CODE) { if (Activity.RESULT_OK == resultCode) { @@ -146,21 +139,9 @@ class SetIdentityServerFragment @Inject constructor( } private fun processIdentityServerChange() { - withState(viewModel) { state -> - if (state.newIdentityServerUrl != null) { - sharedViewModel.requestChangeToIdentityServer(state.newIdentityServerUrl) - parentFragmentManager.popBackStack() - } + viewModel.currentWantedUrl?.let { + sharedViewModel.requestChangeToIdentityServer(it) + parentFragmentManager.popBackStack() } } - - @OnTextChanged(R.id.discovery_identity_server_enter_edittext) - fun onTextEditChange(s: Editable?) { - s?.toString()?.let { viewModel.handle(SetIdentityServerAction.UpdateIdentityServerUrl(it)) } - } - - override fun onResume() { - super.onResume() - (activity as? VectorBaseActivity)?.supportActionBar?.setTitle(R.string.identity_server) - } } diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerState.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerState.kt index bf703fd8e9..2b76d21ce5 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerState.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerState.kt @@ -16,12 +16,10 @@ package im.vector.riotx.features.discovery.change -import androidx.annotation.StringRes import com.airbnb.mvrx.MvRxState data class SetIdentityServerState( - // At first, will contain the default identity server url if any - val newIdentityServerUrl: String? = null, - @StringRes val errorMessageId: Int? = null, - val isVerifyingServer: Boolean = false + val homeServerUrl: String = "", + // Will contain the default identity server url if any + val defaultIdentityServerUrl: String? = null ) : MvRxState diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewEvents.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewEvents.kt index 1addf92d72..8f3b962118 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewEvents.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewEvents.kt @@ -16,10 +16,16 @@ package im.vector.riotx.features.discovery.change +import androidx.annotation.StringRes import im.vector.riotx.core.platform.VectorViewEvents sealed class SetIdentityServerViewEvents : VectorViewEvents { - data class ShowTerms(val newIdentityServer: String) : SetIdentityServerViewEvents() + data class Loading(val message: CharSequence? = null) : SetIdentityServerViewEvents() + data class Failure(@StringRes val errorMessageId: Int) : SetIdentityServerViewEvents() + data class OtherFailure(val failure: Throwable) : SetIdentityServerViewEvents() + + data class ShowTerms(val identityServerUrl: String) : SetIdentityServerViewEvents() + object NoTerms : SetIdentityServerViewEvents() object TermsAccepted : SetIdentityServerViewEvents() } diff --git a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt index 29eea88cc9..29361bda4a 100644 --- a/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/discovery/change/SetIdentityServerViewModel.kt @@ -49,7 +49,8 @@ class SetIdentityServerViewModel @AssistedInject constructor( val session = (viewModelContext.activity as HasScreenInjector).injector().activeSessionHolder().getActiveSession() return SetIdentityServerState( - newIdentityServerUrl = session.identityService().getDefaultIdentityServer() + homeServerUrl = session.sessionParams.homeServerUrl, + defaultIdentityServerUrl = session.identityService().getDefaultIdentityServer() ) } @@ -68,36 +69,37 @@ class SetIdentityServerViewModel @AssistedInject constructor( } } - val userLanguage = stringProvider.getString(R.string.resources_language) + var currentWantedUrl: String? = null + private set + + private val userLanguage = stringProvider.getString(R.string.resources_language) override fun handle(action: SetIdentityServerAction) { when (action) { - is SetIdentityServerAction.UpdateIdentityServerUrl -> updateIdentityServerUrl(action) - SetIdentityServerAction.DoChangeIdentityServerUrl -> doChangeIdentityServerUrl() + SetIdentityServerAction.UseDefaultIdentityServer -> useDefault() + is SetIdentityServerAction.UseCustomIdentityServer -> usedCustomIdentityServerUrl(action) }.exhaustive } - private fun updateIdentityServerUrl(action: SetIdentityServerAction.UpdateIdentityServerUrl) { - setState { - copy( - newIdentityServerUrl = action.url, - errorMessageId = null - ) - } + private fun useDefault() = withState { state -> + state.defaultIdentityServerUrl?.let { doChangeIdentityServerUrl(it) } } - private fun doChangeIdentityServerUrl() = withState { - var baseUrl: String? = it.newIdentityServerUrl - if (baseUrl.isNullOrBlank()) { - setState { - copy(errorMessageId = R.string.settings_discovery_please_enter_server) - } - return@withState + private fun usedCustomIdentityServerUrl(action: SetIdentityServerAction.UseCustomIdentityServer) { + doChangeIdentityServerUrl(action.url) + } + + private fun doChangeIdentityServerUrl(url: String) { + var baseUrl = url + if (baseUrl.isEmpty()) { + _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.settings_discovery_please_enter_server)) + return } baseUrl = sanitatizeBaseURL(baseUrl) - setState { - copy(isVerifyingServer = true) - } + + currentWantedUrl = baseUrl + + _viewEvents.post(SetIdentityServerViewEvents.Loading()) // First ping the identity server v2 API mxSession.identityService().isValidIdentityServer(baseUrl, object : MatrixCallback { @@ -107,15 +109,11 @@ class SetIdentityServerViewModel @AssistedInject constructor( } override fun onFailure(failure: Throwable) { - setState { - copy( - isVerifyingServer = false, - errorMessageId = if (failure is IdentityServiceError.OutdatedIdentityServer) { - R.string.identity_server_error_outdated_identity_server - } else { - R.string.settings_discovery_bad_identity_server - } - ) + if (failure is IdentityServiceError.OutdatedIdentityServer) { + _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.identity_server_error_outdated_identity_server)) + } else { + _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.settings_discovery_bad_identity_server)) + _viewEvents.post(SetIdentityServerViewEvents.OtherFailure(failure)) } } }) @@ -127,9 +125,6 @@ class SetIdentityServerViewModel @AssistedInject constructor( object : MatrixCallback { override fun onSuccess(data: GetTermsResponse) { // has all been accepted? - setState { - copy(isVerifyingServer = false) - } val resp = data.serverResponse val tos = resp.getLocalizedTerms(userLanguage) if (tos.isEmpty()) { @@ -147,19 +142,11 @@ class SetIdentityServerViewModel @AssistedInject constructor( override fun onFailure(failure: Throwable) { if (failure is Failure.OtherServerError && failure.httpCode == 404) { - setState { - copy(isVerifyingServer = false) - } // 404: Same as NoTerms - // TODO Handle the case where identity _viewEvents.post(SetIdentityServerViewEvents.NoTerms) } else { - setState { - copy( - isVerifyingServer = false, - errorMessageId = R.string.settings_discovery_bad_identity_server - ) - } + _viewEvents.post(SetIdentityServerViewEvents.Failure(R.string.settings_discovery_bad_identity_server)) + _viewEvents.post(SetIdentityServerViewEvents.OtherFailure(failure)) } } }) diff --git a/vector/src/main/res/layout/fragment_set_identity_server.xml b/vector/src/main/res/layout/fragment_set_identity_server.xml index 336d89ebb6..20e849292d 100644 --- a/vector/src/main/res/layout/fragment_set_identity_server.xml +++ b/vector/src/main/res/layout/fragment_set_identity_server.xml @@ -3,51 +3,75 @@ xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" - android:layout_height="match_parent"> + android:layout_height="match_parent" + android:background="?riotx_background"> - + android:layout_height="wrap_content" + android:orientation="vertical" + android:padding="16dp"> + + + + + + + app:errorEnabled="true"> + android:maxLines="1" + android:textColor="?riotx_text_primary" /> - - + android:layout_gravity="end" + android:text="@string/identity_server_set_alternative_submit" + android:textAllCaps="false" /> - + \ No newline at end of file diff --git a/vector/src/main/res/values/strings.xml b/vector/src/main/res/values/strings.xml index 6aced9af7f..4f8474760d 100644 --- a/vector/src/main/res/values/strings.xml +++ b/vector/src/main/res/values/strings.xml @@ -1736,7 +1736,7 @@ Not all features in Riot are implemented in RiotX yet. Main missing (and coming We sent you a confirm email to %s, please first check your email and click on the confirmation link Pending - Enter a new identity server + Enter an identity server URL Could not connect to identity server Please enter the identity server url Identity server has no terms of services @@ -2412,4 +2412,10 @@ Not all features in Riot are implemented in RiotX yet. Main missing (and coming The association has failed. The is no current association with this identifier. + Your homeserver (%1$s) proposes to use %2$s for your identity server + Use %1$s + Alternatively, you can enter any other identity server URL + Enter the URL of an identity server + Submit +