From a1aa16715d1410fc568eb06885fd87e27b792af8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 15 Nov 2019 11:47:49 +0100 Subject: [PATCH] Login screens: move elements from ViewState to ViewModel --- .../features/login/AbstractLoginFragment.kt | 4 +- .../riotx/features/login/LoginActivity.kt | 11 ++- .../riotx/features/login/LoginFragment.kt | 55 +++++++------- .../riotx/features/login/LoginNavigation.kt | 2 +- .../login/LoginServerSelectionFragment.kt | 35 +++++---- .../login/LoginServerUrlFormFragment.kt | 73 ++++++++++--------- .../LoginSignUpSignInSelectionFragment.kt | 27 ++++--- .../login/LoginSsoFallbackFragment.kt | 6 +- .../riotx/features/login/LoginViewModel.kt | 26 +++---- .../riotx/features/login/LoginViewState.kt | 2 - 10 files changed, 124 insertions(+), 117 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/login/AbstractLoginFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/AbstractLoginFragment.kt index c0ff5103f9..9e57350b79 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/AbstractLoginFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/AbstractLoginFragment.kt @@ -28,7 +28,7 @@ import im.vector.riotx.core.platform.VectorBaseFragment */ abstract class AbstractLoginFragment : VectorBaseFragment(), OnBackPressed { - protected val viewModel: LoginViewModel by activityViewModel() + protected val loginViewModel: LoginViewModel by activityViewModel() protected lateinit var loginSharedActionViewModel: LoginSharedActionViewModel @CallSuper @@ -44,6 +44,6 @@ abstract class AbstractLoginFragment : VectorBaseFragment(), OnBackPressed { return false } - // Reset any modification of the viewModel by the current fragment + // Reset any modification on the loginViewModel by the current fragment abstract fun resetViewModel() } diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginActivity.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginActivity.kt index 0eaaea33d4..7e90931716 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginActivity.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginActivity.kt @@ -67,7 +67,7 @@ class LoginActivity : VectorBaseActivity() { when (it) { is LoginNavigation.OpenServerSelection -> addFragmentToBackstack(R.id.loginFragmentContainer, LoginServerSelectionFragment::class.java) is LoginNavigation.OnServerSelectionDone -> onServerSelectionDone() - is LoginNavigation.OnSignModeSelected -> onSignModeSelected(it) + is LoginNavigation.OnSignModeSelected -> onSignModeSelected() is LoginNavigation.OnLoginFlowRetrieved -> onLoginFlowRetrieved() is LoginNavigation.OnSsoLoginFallbackError -> onSsoLoginFallbackError(it) } @@ -109,17 +109,16 @@ class LoginActivity : VectorBaseActivity() { .show() } - private fun onServerSelectionDone() = withState(loginViewModel) { - when (it.serverType) { + private fun onServerSelectionDone() { + when (loginViewModel.serverType) { ServerType.MatrixOrg -> Unit // In this case, we wait for the login flow ServerType.Modular, ServerType.Other -> addFragmentToBackstack(R.id.loginFragmentContainer, LoginServerUrlFormFragment::class.java) } } - private fun onSignModeSelected(mode: LoginNavigation.OnSignModeSelected) { - // We cannot use the state to get the SignMode, it is not ready... - when (mode.signMode) { + private fun onSignModeSelected() { + when (loginViewModel.signMode) { SignMode.Unknown -> error("Sign mode has to be set before calling this method") SignMode.SignUp -> Unit // TODO addFragmentToBackstack(R.id.loginFragmentContainer, SignUpFragment::class.java) SignMode.SignIn -> { diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginFragment.kt index 4cd25d7640..2bc5a6171e 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginFragment.kt @@ -49,6 +49,7 @@ class LoginFragment @Inject constructor( override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + setupUi() setupLoginButton() setupPasswordReveal() } @@ -57,7 +58,32 @@ class LoginFragment @Inject constructor( val login = loginField.text?.trim().toString() val password = passwordField.text?.trim().toString() - viewModel.handle(LoginAction.Login(login, password)) + loginViewModel.handle(LoginAction.Login(login, password)) + } + + private fun setupUi() { + when (loginViewModel.serverType) { + ServerType.MatrixOrg -> { + loginServerIcon.isVisible = true + loginServerIcon.setImageResource(R.drawable.ic_logo_matrix_org) + loginTitle.text = getString(R.string.login_connect_to, "matrix.org") + loginNotice.text = getString(R.string.login_server_matrix_org_text) + } + ServerType.Modular -> { + loginServerIcon.isVisible = true + loginServerIcon.setImageResource(R.drawable.ic_logo_modular) + // TODO + loginTitle.text = getString(R.string.login_connect_to, "TODO") + // TODO Remove https:// + loginNotice.text = loginViewModel.getHomeServerUrl() + } + ServerType.Other -> { + loginServerIcon.isVisible = false + loginTitle.text = getString(R.string.login_server_other_title) + // TODO Remove https:// + loginNotice.text = loginViewModel.getHomeServerUrl() + } + } } private fun setupLoginButton() { @@ -109,33 +135,10 @@ class LoginFragment @Inject constructor( } override fun resetViewModel() { - viewModel.handle(LoginAction.ResetLogin) + loginViewModel.handle(LoginAction.ResetLogin) } - override fun invalidate() = withState(viewModel) { state -> - when (state.serverType) { - ServerType.MatrixOrg -> { - loginServerIcon.isVisible = true - loginServerIcon.setImageResource(R.drawable.ic_logo_matrix_org) - loginTitle.text = getString(R.string.login_connect_to, "matrix.org") - loginNotice.text = getString(R.string.login_server_matrix_org_text) - } - ServerType.Modular -> { - loginServerIcon.isVisible = true - loginServerIcon.setImageResource(R.drawable.ic_logo_modular) - // TODO - loginTitle.text = getString(R.string.login_connect_to, "TODO") - // TODO Remove https:// - loginNotice.text = viewModel.getHomeServerUrl() - } - ServerType.Other -> { - loginServerIcon.isVisible = false - loginTitle.text = getString(R.string.login_server_other_title) - // TODO Remove https:// - loginNotice.text = viewModel.getHomeServerUrl() - } - } - + override fun invalidate() = withState(loginViewModel) { state -> when (state.asyncLoginAction) { is Loading -> { // Ensure password is hidden diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginNavigation.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginNavigation.kt index a223660708..2c366a0eb7 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginNavigation.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginNavigation.kt @@ -23,7 +23,7 @@ sealed class LoginNavigation : VectorSharedAction { object OpenServerSelection : LoginNavigation() object OnServerSelectionDone : LoginNavigation() object OnLoginFlowRetrieved : LoginNavigation() - data class OnSignModeSelected(val signMode: SignMode) : LoginNavigation() + object OnSignModeSelected : LoginNavigation() //object OpenSsoLoginFallback : LoginNavigation() data class OnSsoLoginFallbackError(val errorCode: Int, val description: String, val failingUrl: String) : LoginNavigation() } diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginServerSelectionFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginServerSelectionFragment.kt index 27ae99c20b..e819389b9c 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginServerSelectionFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginServerSelectionFragment.kt @@ -38,13 +38,16 @@ class LoginServerSelectionFragment @Inject constructor() : AbstractLoginFragment override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + updateSelectedChoice() initTextViews() } - private fun updateSelectedChoice(serverType: ServerType) { - loginServerChoiceMatrixOrg.isChecked = serverType == ServerType.MatrixOrg - loginServerChoiceModular.isChecked = serverType == ServerType.Modular - loginServerChoiceOther.isChecked = serverType == ServerType.Other + private fun updateSelectedChoice() { + loginViewModel.serverType.let { + loginServerChoiceMatrixOrg.isChecked = it == ServerType.MatrixOrg + loginServerChoiceModular.isChecked = it == ServerType.Modular + loginServerChoiceOther.isChecked = it == ServerType.Other + } } private fun initTextViews() { @@ -56,7 +59,6 @@ class LoginServerSelectionFragment @Inject constructor() : AbstractLoginFragment openUrlInExternalBrowser(requireActivity(), "https://example.org") } } - } @OnClick(R.id.loginServerChoiceMatrixOrg) @@ -65,7 +67,8 @@ class LoginServerSelectionFragment @Inject constructor() : AbstractLoginFragment // Consider this is a submit submit() } else { - viewModel.handle(LoginAction.UpdateServerType(ServerType.MatrixOrg)) + loginViewModel.handle(LoginAction.UpdateServerType(ServerType.MatrixOrg)) + updateSelectedChoice() } } @@ -75,7 +78,8 @@ class LoginServerSelectionFragment @Inject constructor() : AbstractLoginFragment // Consider this is a submit submit() } else { - viewModel.handle(LoginAction.UpdateServerType(ServerType.Modular)) + loginViewModel.handle(LoginAction.UpdateServerType(ServerType.Modular)) + updateSelectedChoice() } } @@ -85,33 +89,32 @@ class LoginServerSelectionFragment @Inject constructor() : AbstractLoginFragment // Consider this is a submit submit() } else { - viewModel.handle(LoginAction.UpdateServerType(ServerType.Other)) + loginViewModel.handle(LoginAction.UpdateServerType(ServerType.Other)) + updateSelectedChoice() } } @OnClick(R.id.loginServerSubmit) - fun submit() = withState(viewModel) { - if (it.serverType == ServerType.MatrixOrg) { + fun submit() { + if (loginViewModel.serverType == ServerType.MatrixOrg) { // Request login flow here - viewModel.handle(LoginAction.UpdateHomeServer(getString(R.string.matrix_org_server_url))) + loginViewModel.handle(LoginAction.UpdateHomeServer(getString(R.string.matrix_org_server_url))) } else { loginSharedActionViewModel.post(LoginNavigation.OnServerSelectionDone) } } override fun resetViewModel() { - viewModel.handle(LoginAction.ResetHomeServerType) + loginViewModel.handle(LoginAction.ResetHomeServerType) } - override fun invalidate() = withState(viewModel) { - updateSelectedChoice(it.serverType) - + override fun invalidate() = withState(loginViewModel) { when (it.asyncHomeServerLoginFlowRequest) { is Fail -> { // TODO Display error in a dialog? } is Success -> { - // The home server url is valid + // LoginFlow for matrix.org has been retrieved loginSharedActionViewModel.post(LoginNavigation.OnLoginFlowRetrieved) } } diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginServerUrlFormFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginServerUrlFormFragment.kt index b5f002b353..c3d841369b 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginServerUrlFormFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginServerUrlFormFragment.kt @@ -44,6 +44,11 @@ class LoginServerUrlFormFragment @Inject constructor( override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + setupUi() + setupHomeServerField() + } + + private fun setupHomeServerField() { // TODO Import code from Riot to clear error on TIL loginServerUrlFormHomeServerUrl.textChanges() .subscribe( @@ -64,39 +69,8 @@ class LoginServerUrlFormFragment @Inject constructor( } } - @OnClick(R.id.loginServerUrlFormLearnMore) - fun learMore() { - // TODO - openUrlInExternalBrowser(requireActivity(), "https://example.org") - } - - override fun resetViewModel() { - viewModel.handle(LoginAction.ResetHomeServerUrl) - } - - @SuppressLint("SetTextI18n") - @OnClick(R.id.loginServerUrlFormSubmit) - fun submit() { - // Static check of homeserver url, empty, malformed, etc. - var serverUrl = loginServerUrlFormHomeServerUrl.text.toString() - - when { - serverUrl.isBlank() -> { - loginServerUrlFormHomeServerUrlTil.error = getString(R.string.login_error_invalid_home_server) - } - else -> { - if (serverUrl.startsWith("http").not()) { - serverUrl = "https://$serverUrl" - loginServerUrlFormHomeServerUrl.setText(serverUrl) - - } - viewModel.handle(LoginAction.UpdateHomeServer(serverUrl)) - } - } - } - - override fun invalidate() = withState(viewModel) { state -> - when (state.serverType) { + private fun setupUi() { + when (loginViewModel.serverType) { ServerType.Modular -> { loginServerUrlFormIcon.isVisible = true loginServerUrlFormTitle.text = getString(R.string.login_connect_to_modular) @@ -115,7 +89,40 @@ class LoginServerUrlFormFragment @Inject constructor( } else -> error("This fragment should not be display in matrix.org mode") } + } + @OnClick(R.id.loginServerUrlFormLearnMore) + fun learMore() { + // TODO + openUrlInExternalBrowser(requireActivity(), "https://example.org") + } + + override fun resetViewModel() { + loginViewModel.handle(LoginAction.ResetHomeServerUrl) + } + + @SuppressLint("SetTextI18n") + @OnClick(R.id.loginServerUrlFormSubmit) + fun submit() { + // Static check of homeserver url, empty, malformed, etc. + var serverUrl = loginServerUrlFormHomeServerUrl.text.toString() + + when { + serverUrl.isBlank() -> { + loginServerUrlFormHomeServerUrlTil.error = getString(R.string.login_error_invalid_home_server) + } + else -> { + if (serverUrl.startsWith("http").not()) { + serverUrl = "https://$serverUrl" + loginServerUrlFormHomeServerUrl.setText(serverUrl) + + } + loginViewModel.handle(LoginAction.UpdateHomeServer(serverUrl)) + } + } + } + + override fun invalidate() = withState(loginViewModel) { state -> when (state.asyncHomeServerLoginFlowRequest) { is Fail -> { // TODO Error text is not correct diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginSignUpSignInSelectionFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginSignUpSignInSelectionFragment.kt index 23c94425bc..cdb9aa8b7a 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginSignUpSignInSelectionFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginSignUpSignInSelectionFragment.kt @@ -16,9 +16,10 @@ package im.vector.riotx.features.login +import android.os.Bundle +import android.view.View import androidx.core.view.isVisible import butterknife.OnClick -import com.airbnb.mvrx.withState import im.vector.riotx.R import kotlinx.android.synthetic.main.fragment_login_signup_signin_selection.* import javax.inject.Inject @@ -30,8 +31,14 @@ class LoginSignUpSignInSelectionFragment @Inject constructor() : AbstractLoginFr override fun getLayoutResId() = R.layout.fragment_login_signup_signin_selection - private fun updateViews(serverType: ServerType) { - when (serverType) { + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + setupUi() + } + + private fun setupUi() { + when (loginViewModel.serverType) { ServerType.MatrixOrg -> { loginSignupSigninServerIcon.setImageResource(R.drawable.ic_logo_matrix_org) loginSignupSigninServerIcon.isVisible = true @@ -54,21 +61,17 @@ class LoginSignUpSignInSelectionFragment @Inject constructor() : AbstractLoginFr @OnClick(R.id.loginSignupSigninSignUp) fun signUp() { - viewModel.handle(LoginAction.UpdateSignMode(SignMode.SignUp)) - loginSharedActionViewModel.post(LoginNavigation.OnSignModeSelected(SignMode.SignUp)) + loginViewModel.handle(LoginAction.UpdateSignMode(SignMode.SignUp)) + loginSharedActionViewModel.post(LoginNavigation.OnSignModeSelected) } @OnClick(R.id.loginSignupSigninSignIn) fun signIn() { - viewModel.handle(LoginAction.UpdateSignMode(SignMode.SignIn)) - loginSharedActionViewModel.post(LoginNavigation.OnSignModeSelected(SignMode.SignIn)) + loginViewModel.handle(LoginAction.UpdateSignMode(SignMode.SignIn)) + loginSharedActionViewModel.post(LoginNavigation.OnSignModeSelected) } override fun resetViewModel() { - viewModel.handle(LoginAction.ResetSignMode) - } - - override fun invalidate() = withState(viewModel) { - updateViews(it.serverType) + loginViewModel.handle(LoginAction.ResetSignMode) } } diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginSsoFallbackFragment.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginSsoFallbackFragment.kt index 36ab47bb55..fcf34bbdc4 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginSsoFallbackFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginSsoFallbackFragment.kt @@ -74,7 +74,7 @@ class LoginSsoFallbackFragment @Inject constructor() : AbstractLoginFragment() { // the user agent to bypass the limitation of Google, as a quick fix (a proper solution will be to use the SSO SDK) login_sso_fallback_webview.settings.userAgentString = "Mozilla/5.0 Google" - homeServerUrl = viewModel.getHomeServerUrl() + homeServerUrl = loginViewModel.getHomeServerUrl() if (!homeServerUrl.endsWith("/")) { homeServerUrl += "/" @@ -248,7 +248,7 @@ class LoginSsoFallbackFragment @Inject constructor() : AbstractLoginFragment() { refreshToken = null ) - viewModel.handle(LoginAction.SsoLoginSuccess(safeCredentials)) + loginViewModel.handle(LoginAction.SsoLoginSuccess(safeCredentials)) } } } catch (e: Exception) { @@ -273,7 +273,7 @@ class LoginSsoFallbackFragment @Inject constructor() : AbstractLoginFragment() { refreshToken = null ) - viewModel.handle(LoginAction.SsoLoginSuccess(credentials)) + loginViewModel.handle(LoginAction.SsoLoginSuccess(credentials)) } } } diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginViewModel.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginViewModel.kt index 96f8d5ea99..f4a9b24812 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginViewModel.kt @@ -55,6 +55,12 @@ class LoginViewModel @AssistedInject constructor(@Assisted initialState: LoginVi } } + var serverType: ServerType = ServerType.MatrixOrg + private set + + var signMode: SignMode = SignMode.Unknown + private set + private var loginConfig: LoginConfig? = null private var homeServerConnectionConfig: HomeServerConnectionConfig? = null @@ -93,16 +99,12 @@ class LoginViewModel @AssistedInject constructor(@Assisted initialState: LoginVi } } LoginAction.ResetHomeServerType -> { - setState { - copy( - serverType = ServerType.MatrixOrg - ) - } + serverType = ServerType.MatrixOrg } LoginAction.ResetSignMode -> { + signMode = SignMode.Unknown setState { copy( - signMode = SignMode.Unknown, asyncHomeServerLoginFlowRequest = Uninitialized ) } @@ -111,19 +113,11 @@ class LoginViewModel @AssistedInject constructor(@Assisted initialState: LoginVi } private fun handleUpdateSignMode(action: LoginAction.UpdateSignMode) { - setState { - copy( - signMode = action.signMode - ) - } + signMode = action.signMode } private fun handleUpdateServerType(action: LoginAction.UpdateServerType) { - setState { - copy( - serverType = action.serverType - ) - } + serverType = action.serverType } private fun handleInitWith(action: LoginAction.InitWith) { diff --git a/vector/src/main/java/im/vector/riotx/features/login/LoginViewState.kt b/vector/src/main/java/im/vector/riotx/features/login/LoginViewState.kt index 4be96d20c2..0853765d63 100644 --- a/vector/src/main/java/im/vector/riotx/features/login/LoginViewState.kt +++ b/vector/src/main/java/im/vector/riotx/features/login/LoginViewState.kt @@ -19,8 +19,6 @@ package im.vector.riotx.features.login import com.airbnb.mvrx.* data class LoginViewState( - val serverType: ServerType = ServerType.MatrixOrg, - val signMode: SignMode = SignMode.Unknown, val asyncLoginAction: Async = Uninitialized, val asyncHomeServerLoginFlowRequest: Async = Uninitialized ) : MvRxState {