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

* Improve `ButtonWithProgress` component.

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

changelog.d/235.misc

@@ -0,0 +1 @@
+Improve ButtonWithProgress component, replace login and change server buttons

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()

features/verifysession/impl/src/main/kotlin/io/element/android/features/verifysession/impl/VerifySelfSessionView.kt

@@ -56,7 +56,6 @@ import io.element.android.libraries.designsystem.components.button.ButtonWithPro
 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.Surface
@@ -257,21 +256,12 @@ internal fun BottomMenu(screenState: VerifySelfSessionState, goBack: () -> Unit)
             .padding(horizontal = 20.dp),
         horizontalAlignment = Alignment.CenterHorizontally
     ) {
-        if (isVerifying) {
-            ButtonWithProgress(
-                modifier = Modifier.fillMaxWidth(),
-                onClick = { positiveButtonEvent?.let { eventSink(it) } }
-            ) {
-                positiveButtonTitle?.let { Text(stringResource(it), style = ElementTextStyles.Button) }
-            }
-        } else {
-            Button(
-                modifier = Modifier.fillMaxWidth(),
-                onClick = { positiveButtonEvent?.let { eventSink(it) } }
-            ) {
-                positiveButtonTitle?.let { Text(stringResource(it), style = ElementTextStyles.Button) }
-            }
-        }
+        ButtonWithProgress(
+            text = positiveButtonTitle?.let { stringResource(it) },
+            showProgress = isVerifying,
+            modifier = Modifier.fillMaxWidth(),
+            onClick = { positiveButtonEvent?.let { eventSink(it) } }
+        )
         if (negativeButtonTitle != null) {
             Spacer(modifier = Modifier.height(16.dp))
             TextButton(

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/button/ButtonWithProgress.kt

@@ -19,7 +19,6 @@ package io.element.android.libraries.designsystem.components.button
 import androidx.compose.foundation.BorderStroke
 import androidx.compose.foundation.interaction.MutableInteractionSource
 import androidx.compose.foundation.layout.PaddingValues
-import androidx.compose.foundation.layout.RowScope
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.width
@@ -33,6 +32,7 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Shape
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
+import io.element.android.libraries.designsystem.ElementTextStyles
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.theme.components.Button
@@ -40,10 +40,20 @@ import io.element.android.libraries.designsystem.theme.components.CircularProgre
 import io.element.android.libraries.designsystem.theme.components.ElementButtonDefaults
 import io.element.android.libraries.designsystem.theme.components.Text
 
+/**
+ * A component that will display a button with an indeterminate circular progressbar.
+ * When [showProgress] is true:
+ *  - A circular progressbar is displayed.
+ *  - [text] is replaced by [progressText], if defined.
+ *  - [onClick] gets disabled.
+ */
 @Composable
 fun ButtonWithProgress(
+    text: String?,
     onClick: () -> Unit,
     modifier: Modifier = Modifier,
+    showProgress: Boolean = false,
+    progressText: String? = text,
     enabled: Boolean = true,
     shape: Shape = ElementButtonDefaults.shape,
     colors: ButtonColors = ElementButtonDefaults.buttonColors(),
@@ -51,10 +61,11 @@ fun ButtonWithProgress(
     border: BorderStroke? = null,
     contentPadding: PaddingValues = ElementButtonDefaults.ContentPadding,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    content: @Composable RowScope.() -> Unit
 ) {
     Button(
-        onClick = onClick,
+        onClick = {
+            if (!showProgress) { onClick() }
+        },
         modifier = modifier,
         enabled = enabled,
         shape = shape,
@@ -64,15 +75,21 @@ fun ButtonWithProgress(
         contentPadding = contentPadding,
         interactionSource = interactionSource,
     ) {
-        CircularProgressIndicator(
-            modifier = Modifier
-                .progressSemantics()
-                .size(18.dp),
-            color = MaterialTheme.colorScheme.onPrimary,
-            strokeWidth = 2.dp,
-        )
-        Spacer(Modifier.width(10.dp))
-        content()
+        if (showProgress) {
+            CircularProgressIndicator(
+                modifier = Modifier
+                    .progressSemantics()
+                    .size(18.dp),
+                color = MaterialTheme.colorScheme.onPrimary,
+                strokeWidth = 2.dp,
+            )
+            if (progressText != null) {
+                Spacer(Modifier.width(10.dp))
+                Text(progressText, style = ElementTextStyles.Button)
+            }
+        } else if (text != null) {
+            Text(text, style = ElementTextStyles.Button)
+        }
     }
 }
 
@@ -86,7 +103,9 @@ internal fun ButtonWithProgressDarkPreview() = ElementPreviewDark { ContentToPre
 
 @Composable
 private fun ContentToPreview() {
-    ButtonWithProgress(onClick = {}) {
-        Text(text = "Button with progress")
-    }
+    ButtonWithProgress(
+        text = "Button with progress",
+        onClick = {},
+        showProgress = true,
+    )
 }

libraries/ui-strings/src/main/res/values/localazy.xml

@@ -122,10 +122,16 @@
   <string name="rageshake_dialog_content">"You seem to be shaking the phone in frustration. Would you like to open the bug report screen?"</string>
   <string name="report_content_explanation">"Reporting this message will send it’s unique ‘event ID’ to the administrator of your homeserver. If messages in this room are encrypted, your homeserver administrator will not be able to read the message text or view any files or images."</string>
   <string name="report_content_hint">"Reason for reporting this content"</string>
-  <string name="room_details_encryption_enabled_description">"Messages in this room are end-to-end encrypted. Learn more &amp; verify users in their profile."</string>
   <string name="room_timeline_beginning_of_room">"This is the beginning of %1$s."</string>
   <string name="room_timeline_beginning_of_room_no_name">"This is the beginning of this conversation."</string>
   <string name="room_timeline_read_marker_title">"New"</string>
+  <string name="screen_room_details_encryption_enabled_subtitle">"Messages are secured with locks. Only you and the recipients have the unique keys to unlock them."</string>
+  <string name="screen_room_details_encryption_enabled_title">"Message encryption enabled"</string>
+  <string name="screen_room_details_invite_people_title">"Invite people"</string>
+  <string name="screen_room_details_leave_room_title">"Leave room"</string>
+  <string name="screen_room_details_people_title">"People"</string>
+  <string name="screen_room_details_security_title">"Security"</string>
+  <string name="screen_room_details_topic_title">"Topic"</string>
   <string name="settings_rageshake">"Rageshake"</string>
   <string name="settings_rageshake_detection_threshold">"Detection threshold"</string>
   <string name="settings_title_general">"General"</string>