Improve ButtonWithProgress component, replace login and change server buttons (#235)

* Improve `ButtonWithProgress` component.

* Replace `CircularProgresIndicator` with `ButtonWithProgress` in login and server selection screens.

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerState.kt

@@ -23,5 +23,5 @@ data class ChangeServerState(
     val changeServerAction: Async<Unit>,
     val eventSink: (ChangeServerEvents) -> Unit,
 ) {
-    val submitEnabled = homeserver.isNotEmpty() && changeServerAction is Async.Uninitialized
+    val submitEnabled: Boolean get() = homeserver.isNotEmpty() && (changeServerAction is Async.Uninitialized || changeServerAction is Async.Loading)
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt

@@ -64,14 +64,12 @@ import io.element.android.libraries.designsystem.ElementTextStyles
 import io.element.android.libraries.designsystem.LinkColor
 import io.element.android.libraries.designsystem.components.ClickableLinkText
 import io.element.android.libraries.designsystem.components.button.BackButton
+import io.element.android.libraries.designsystem.components.button.ButtonWithProgress
 import io.element.android.libraries.designsystem.components.dialogs.ConfirmationDialog
-import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
 import io.element.android.libraries.designsystem.components.form.textFieldState
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.theme.LocalColors
-import io.element.android.libraries.designsystem.theme.components.Button
-import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.Icon
 import io.element.android.libraries.designsystem.theme.components.IconButton
 import io.element.android.libraries.designsystem.theme.components.Scaffold
@@ -94,9 +92,9 @@ fun ChangeServerView(
 ) {
     val eventSink = state.eventSink
     val scrollState = rememberScrollState()
-    val interactionEnabled by remember(state.changeServerAction) {
+    val isLoading by remember(state.changeServerAction) {
         derivedStateOf {
-            state.changeServerAction !is Async.Loading
+            state.changeServerAction is Async.Loading
         }
     }
     val invalidHomeserverError = (state.changeServerAction as? Async.Failure)?.error as? ChangeServerError.InlineErrorMessage
@@ -114,7 +112,7 @@ fun ChangeServerView(
         topBar = {
             TopAppBar(
                 title = {},
-                navigationIcon = { BackButton(onClick = onBackPressed, enabled = interactionEnabled) }
+                navigationIcon = { BackButton(onClick = onBackPressed) }
             )
         }
     ) { padding ->
@@ -179,7 +177,7 @@ fun ChangeServerView(
                 var homeserverFieldState by textFieldState(stateValue = state.homeserver)
                 TextField(
                     value = homeserverFieldState,
-                    readOnly = !interactionEnabled,
+                    readOnly = isLoading,
                     modifier = Modifier
                         .fillMaxWidth()
                         .testTag(TestTags.changeServerServer)
@@ -201,7 +199,7 @@ fun ChangeServerView(
                         {
                             IconButton(onClick = {
                                 eventSink(ChangeServerEvents.SetServer(""))
-                            }, enabled = interactionEnabled) {
+                            }, enabled = !isLoading) {
                                 Icon(imageVector = Icons.Filled.Close, contentDescription = stringResource(StringR.string.action_clear))
                             }
                         }
@@ -244,37 +242,23 @@ fun ChangeServerView(
                     })
                 }
                 Spacer(Modifier.height(32.dp))
-                Button(
+                ButtonWithProgress(
+                    text = stringResource(id = R.string.screen_change_server_submit),
+                    showProgress = isLoading,
                     onClick = ::submit,
-                    enabled = interactionEnabled && state.submitEnabled,
+                    enabled = state.submitEnabled,
                     modifier = Modifier
                         .fillMaxWidth()
                         .testTag(TestTags.changeServerContinue)
-                ) {
-                    Text(text = stringResource(id = R.string.screen_change_server_submit), style = ElementTextStyles.Button)
-                }
+                )
                 if (state.changeServerAction is Async.Success) {
                     onChangeServerSuccess()
                 }
             }
-            if (state.changeServerAction is Async.Loading) {
-                CircularProgressIndicator(
-                    modifier = Modifier.align(Alignment.Center)
-                )
-            }
         }
     }
 }
 
-@Composable
-internal fun ChangeServerErrorDialog(title: String, message: String, onDismiss: () -> Unit) {
-    ErrorDialog(
-        title = title,
-        content = message,
-        onDismiss = onDismiss
-    )
-}
-
 @Composable
 internal fun SlidingSyncNotSupportedDialog(onLearnMoreClicked: () -> Unit, onDismiss: () -> Unit) {
     ConfirmationDialog(

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/root/LoginRootState.kt

@@ -27,8 +27,8 @@ data class LoginRootState(
     val formState: LoginFormState,
     val eventSink: (LoginRootEvents) -> Unit
 ) {
-    val submitEnabled =
-        formState.login.isNotEmpty() && formState.password.isNotEmpty() && loggedInState != LoggedInState.LoggingIn
+    val submitEnabled: Boolean get() =
+        formState.login.isNotEmpty() && formState.password.isNotEmpty() && loggedInState !is LoggedInState.ErrorLoggingIn
 }
 
 sealed interface LoggedInState {

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/root/LoginRootView.kt

@@ -65,13 +65,12 @@ import androidx.compose.ui.unit.dp
 import io.element.android.features.login.impl.R
 import io.element.android.features.login.impl.error.loginError
 import io.element.android.libraries.designsystem.ElementTextStyles
+import io.element.android.libraries.designsystem.components.button.BackButton
+import io.element.android.libraries.designsystem.components.button.ButtonWithProgress
 import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
 import io.element.android.libraries.designsystem.components.form.textFieldState
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
-import io.element.android.libraries.designsystem.components.button.BackButton
-import io.element.android.libraries.designsystem.theme.components.Button
-import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.Icon
 import io.element.android.libraries.designsystem.theme.components.IconButton
 import io.element.android.libraries.designsystem.theme.components.Scaffold
@@ -93,16 +92,16 @@ fun LoginRootView(
     onLoginWithSuccess: (SessionId) -> Unit = {},
     onBackPressed: () -> Unit,
 ) {
-    val interactionEnabled by remember(state.loggedInState) {
+    val isLoading by remember(state.loggedInState) {
         derivedStateOf {
-            state.loggedInState != LoggedInState.LoggingIn
+            state.loggedInState == LoggedInState.LoggingIn
         }
     }
     Scaffold(
         topBar = {
             TopAppBar(
                 title = {},
-                navigationIcon = { BackButton(onClick = onBackPressed, enabled = interactionEnabled) },
+                navigationIcon = { BackButton(onClick = onBackPressed) },
             )
         }
     ) { padding ->
@@ -131,14 +130,14 @@ fun LoginRootView(
                 Spacer(Modifier.height(32.dp))
 
                 ChangeServerSection(
-                    interactionEnabled = interactionEnabled,
+                    interactionEnabled = !isLoading,
                     homeserver = state.homeserverDetails.url,
                     onChangeServer = onChangeServer
                 )
 
                 Spacer(Modifier.height(32.dp))
 
-                LoginForm(state = state, interactionEnabled = interactionEnabled)
+                LoginForm(state = state, isLoading = isLoading)
 
                 Spacer(modifier = Modifier.height(32.dp))
 
@@ -147,12 +146,6 @@ fun LoginRootView(
                 is LoggedInState.LoggedIn -> onLoginWithSuccess(loggedInState.sessionId)
                 else -> Unit
             }
-
-            if (state.loggedInState is LoggedInState.LoggingIn) {
-                CircularProgressIndicator(
-                    modifier = Modifier.align(Alignment.Center)
-                )
-            }
         }
     }
 
@@ -216,7 +209,7 @@ internal fun ChangeServerSection(
 @Composable
 internal fun LoginForm(
     state: LoginRootState,
-    interactionEnabled: Boolean,
+    isLoading: Boolean,
     modifier: Modifier = Modifier
 ) {
     var loginFieldState by textFieldState(stateValue = state.formState.login)
@@ -242,7 +235,7 @@ internal fun LoginForm(
         Spacer(modifier = Modifier.height(8.dp))
         TextField(
             value = loginFieldState,
-            readOnly = !interactionEnabled,
+            readOnly = isLoading,
             modifier = Modifier
                 .fillMaxWidth()
                 .onTabOrEnterKeyFocusNext(focusManager)
@@ -282,7 +275,7 @@ internal fun LoginForm(
         Spacer(Modifier.height(20.dp))
         TextField(
             value = passwordFieldState,
-            readOnly = !interactionEnabled,
+            readOnly = isLoading,
             modifier = Modifier
                 .fillMaxWidth()
                 .onTabOrEnterKeyFocusNext(focusManager)
@@ -318,15 +311,15 @@ internal fun LoginForm(
         Spacer(Modifier.height(28.dp))
 
         // Submit
-        Button(
+        ButtonWithProgress(
+            text = stringResource(R.string.screen_login_submit),
+            showProgress = isLoading,
             onClick = ::submit,
-            enabled = interactionEnabled && state.submitEnabled,
+            enabled = state.submitEnabled,
             modifier = Modifier
                 .fillMaxWidth()
                 .testTag(TestTags.loginContinue)
-        ) {
-            Text(text = stringResource(R.string.screen_login_submit), style = ElementTextStyles.Button)
-        }
+        )
     }
 }
 

features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt

@@ -22,8 +22,6 @@ import app.cash.molecule.RecompositionClock
 import app.cash.molecule.moleculeFlow
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
-import io.element.android.features.login.impl.changeserver.ChangeServerEvents
-import io.element.android.features.login.impl.changeserver.ChangeServerPresenter
 import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.matrix.test.A_HOMESERVER
 import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
@@ -34,6 +32,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
+@OptIn(ExperimentalCoroutinesApi::class)
 class ChangeServerPresenterTest {
     @Test
     fun `present - should start with default homeserver`() = runTest {
@@ -92,7 +91,7 @@ class ChangeServerPresenterTest {
             val initialState = awaitItem()
             initialState.eventSink.invoke(ChangeServerEvents.Submit)
             val loadingState = awaitItem()
-            assertThat(loadingState.submitEnabled).isFalse()
+            assertThat(loadingState.submitEnabled).isTrue()
             assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
             val successState = awaitItem()
             assertThat(successState.submitEnabled).isFalse()
@@ -114,7 +113,7 @@ class ChangeServerPresenterTest {
             awaitItem()
             initialState.eventSink.invoke(ChangeServerEvents.Submit)
             val loadingState = awaitItem()
-            assertThat(loadingState.submitEnabled).isFalse()
+            assertThat(loadingState.submitEnabled).isTrue()
             assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
             awaitItem() // Skip changing the url to the parsed domain
             val successState = awaitItem()