Iterate on QrCode login error buttons (#6101)

* Iterate on login error: add a cancel button that fully close the flow.

tom

* Fix compilation warning: `Name contains character(s) that can cause problems on Windows: "`

* Update screenshots

---------

Co-authored-by: ElementBot <android@element.io>

features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/LinkNewDeviceFlowNode.kt

@@ -193,7 +193,8 @@ class LinkNewDeviceFlowNode(
             is ErrorType.Unknown -> ErrorScreenType.UnknownError
             is ErrorType.UnsupportedProtocol -> ErrorScreenType.UnknownError
         }
-        // It is OK to push on backstack, since when user leaves the error screen, a new root will be set
+        // It is OK to push on backstack, since when user leaves the error screen, a new root will be set,
+        // or the whole flow will be popped.
         backstack.push(NavTarget.Error(error))
     }
 
@@ -263,6 +264,12 @@ class LinkNewDeviceFlowNode(
                         linkNewDesktopHandler.reset()
                         backstack.newRoot(NavTarget.Root)
                     }
+
+                    override fun onCancel() {
+                        linkNewMobileHandler.reset()
+                        linkNewDesktopHandler.reset()
+                        callback.onDone()
+                    }
                 }
                 createNode<ErrorNode>(buildContext, listOf(callback, navTarget.errorScreenType))
             }

features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorNode.kt

@@ -27,6 +27,7 @@ class ErrorNode(
 ) : Node(buildContext = buildContext, plugins = plugins) {
     interface Callback : Plugin {
         fun onRetry()
+        fun onCancel()
     }
 
     private val callback: Callback = callback()
@@ -38,6 +39,7 @@ class ErrorNode(
             modifier = modifier,
             errorScreenType = errorScreenType,
             onRetry = callback::onRetry,
+            onCancel = callback::onCancel,
         )
     }
 }

features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorView.kt

@@ -33,6 +33,7 @@ import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.LocalBuildMeta
 import io.element.android.libraries.designsystem.theme.components.Button
+import io.element.android.libraries.designsystem.theme.components.OutlinedButton
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.collections.immutable.persistentListOf
@@ -41,17 +42,23 @@ import kotlinx.collections.immutable.persistentListOf
 fun ErrorView(
     errorScreenType: ErrorScreenType,
     onRetry: () -> Unit,
+    onCancel: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
     val appName = LocalBuildMeta.current.applicationName
-    BackHandler(onBack = onRetry)
+    BackHandler(onBack = onCancel)
     FlowStepPage(
         modifier = modifier,
         iconStyle = BigIcon.Style.AlertSolid,
         title = titleText(errorScreenType, appName),
         subTitle = subtitleText(errorScreenType, appName),
         content = { Content(errorScreenType) },
-        buttons = { Buttons(onRetry) },
+        buttons = {
+            Buttons(
+                onRetry = onRetry,
+                onCancel = onCancel,
+            )
+        },
     )
 }
 
@@ -118,11 +125,19 @@ private fun Content(errorScreenType: ErrorScreenType) {
 }
 
 @Composable
-private fun Buttons(onRetry: () -> Unit) {
+private fun Buttons(
+    onRetry: () -> Unit,
+    onCancel: () -> Unit,
+) {
     Button(
         modifier = Modifier.fillMaxWidth(),
-        text = stringResource(CommonStrings.action_start_over),
-        onClick = onRetry
+        text = stringResource(CommonStrings.action_try_again),
+        onClick = onRetry,
+    )
+    OutlinedButton(
+        modifier = Modifier.fillMaxWidth(),
+        text = stringResource(CommonStrings.action_cancel),
+        onClick = onCancel,
     )
 }
 
@@ -133,6 +148,7 @@ internal fun ErrorViewPreview(@PreviewParameter(ErrorScreenTypeProvider::class)
         ErrorView(
             errorScreenType = errorScreenType,
             onRetry = {},
+            onCancel = {},
         )
     }
 }

features/linknewdevice/impl/src/test/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorViewTest.kt

@@ -12,6 +12,7 @@ import androidx.compose.ui.test.junit4.AndroidComposeTestRule
 import androidx.compose.ui.test.junit4.createAndroidComposeRule
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.element.android.libraries.ui.strings.CommonStrings
+import io.element.android.tests.testutils.EnsureNeverCalled
 import io.element.android.tests.testutils.clickOn
 import io.element.android.tests.testutils.ensureCalledOnce
 import io.element.android.tests.testutils.pressBackKey
@@ -26,33 +27,45 @@ class ErrorViewTest {
     val rule = createAndroidComposeRule<ComponentActivity>()
 
     @Test
-    fun `on back pressed - calls the onRetry callback`() {
+    fun `on back pressed - calls the onCancel callback`() {
         ensureCalledOnce { callback ->
             rule.setErrorView(
-                onRetry = callback
+                onCancel = callback,
             )
             rule.pressBackKey()
         }
     }
 
     @Test
-    fun `on start over button clicked - calls the expected callback`() {
+    fun `on try again button clicked - calls the expected callback`() {
         ensureCalledOnce { callback ->
             rule.setErrorView(
                 onRetry = callback
             )
-            rule.clickOn(CommonStrings.action_start_over)
+            rule.clickOn(CommonStrings.action_try_again)
+        }
+    }
+
+    @Test
+    fun `on cancel button clicked - calls the expected callback`() {
+        ensureCalledOnce { callback ->
+            rule.setErrorView(
+                onCancel = callback
+            )
+            rule.clickOn(CommonStrings.action_cancel)
         }
     }
 
     private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setErrorView(
-        onRetry: () -> Unit,
+        onRetry: () -> Unit = EnsureNeverCalled(),
+        onCancel: () -> Unit = EnsureNeverCalled(),
         errorScreenType: ErrorScreenType = ErrorScreenType.UnknownError,
     ) {
         setContent {
             ErrorView(
                 errorScreenType = errorScreenType,
                 onRetry = onRetry,
+                onCancel = onCancel,
             )
         }
     }

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

@@ -20,6 +20,7 @@ import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.plugin.Plugin
 import com.bumble.appyx.navmodel.backstack.BackStack
+import com.bumble.appyx.navmodel.backstack.operation.pop
 import com.bumble.appyx.navmodel.backstack.operation.push
 import com.bumble.appyx.navmodel.backstack.operation.singleTop
 import dev.zacsweers.metro.AppScope
@@ -196,7 +197,12 @@ class LoginFlowNode(
                 createNode<ChooseAccountProviderNode>(buildContext, listOf(callback))
             }
             NavTarget.QrCode -> {
-                createNode<QrCodeLoginFlowNode>(buildContext)
+                val callback = object : QrCodeLoginFlowNode.Callback {
+                    override fun navigateBack() {
+                        backstack.pop()
+                    }
+                }
+                createNode<QrCodeLoginFlowNode>(buildContext, listOf(callback))
             }
             is NavTarget.ConfirmAccountProvider -> {
                 val inputs = ConfirmAccountProviderNode.Inputs(

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/qrcode/QrCodeLoginFlowNode.kt

@@ -37,6 +37,7 @@ import io.element.android.libraries.architecture.BackstackView
 import io.element.android.libraries.architecture.BaseFlowNode
 import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.bindings
+import io.element.android.libraries.architecture.callback
 import io.element.android.libraries.architecture.createNode
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.di.DependencyInjectionGraphOwner
@@ -64,6 +65,12 @@ class QrCodeLoginFlowNode(
     buildContext = buildContext,
     plugins = plugins,
 ), DependencyInjectionGraphOwner {
+    interface Callback : Plugin {
+        fun navigateBack()
+    }
+
+    private val callback: Callback = callback()
+
     private var authenticationJob: Job? = null
 
     override val graph = qrCodeLoginGraphFactory.create()
@@ -85,7 +92,6 @@ class QrCodeLoginFlowNode(
 
     override fun onBuilt() {
         super.onBuilt()
-
         observeLoginStep()
     }
 
@@ -178,7 +184,13 @@ class QrCodeLoginFlowNode(
             }
             is NavTarget.Error -> {
                 val callback = object : QrCodeErrorNode.Callback {
-                    override fun onRetry() = reset()
+                    override fun onRetry() {
+                        reset()
+                    }
+
+                    override fun onCancel() {
+                        callback.navigateBack()
+                    }
                 }
                 createNode<QrCodeErrorNode>(buildContext, plugins = listOf(navTarget.errorType, callback))
             }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorNode.kt

@@ -31,6 +31,7 @@ class QrCodeErrorNode(
 ) : Node(buildContext = buildContext, plugins = plugins) {
     interface Callback : Plugin {
         fun onRetry()
+        fun onCancel()
     }
 
     private val callback: Callback = callback()
@@ -43,6 +44,7 @@ class QrCodeErrorNode(
             errorScreenType = qrCodeErrorScreenType,
             appName = buildMeta.productionApplicationName,
             onRetry = callback::onRetry,
+            onCancel = callback::onCancel,
         )
     }
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorView.kt

@@ -35,6 +35,7 @@ import io.element.android.libraries.designsystem.components.BigIcon
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.components.Button
+import io.element.android.libraries.designsystem.theme.components.OutlinedButton
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.collections.immutable.persistentListOf
@@ -44,16 +45,22 @@ fun QrCodeErrorView(
     errorScreenType: QrCodeErrorScreenType,
     appName: String,
     onRetry: () -> Unit,
+    onCancel: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    BackHandler(onBack = onRetry)
+    BackHandler(onBack = onCancel)
     FlowStepPage(
         modifier = modifier,
         iconStyle = BigIcon.Style.AlertSolid,
         title = titleText(errorScreenType, appName),
         subTitle = subtitleText(errorScreenType, appName),
         content = { Content(errorScreenType) },
-        buttons = { Buttons(onRetry) },
+        buttons = {
+            Buttons(
+                onRetry = onRetry,
+                onCancel = onCancel,
+            )
+        },
     )
 }
 
@@ -118,11 +125,19 @@ private fun Content(errorScreenType: QrCodeErrorScreenType) {
 }
 
 @Composable
-private fun Buttons(onRetry: () -> Unit) {
+private fun Buttons(
+    onRetry: () -> Unit,
+    onCancel: () -> Unit,
+) {
     Button(
         modifier = Modifier.fillMaxWidth(),
-        text = stringResource(R.string.screen_qr_code_login_start_over_button),
-        onClick = onRetry
+        text = stringResource(CommonStrings.action_try_again),
+        onClick = onRetry,
+    )
+    OutlinedButton(
+        modifier = Modifier.fillMaxWidth(),
+        text = stringResource(CommonStrings.action_cancel),
+        onClick = onCancel,
     )
 }
 
@@ -133,7 +148,8 @@ internal fun QrCodeErrorViewPreview(@PreviewParameter(QrCodeErrorScreenTypeProvi
         QrCodeErrorView(
             errorScreenType = errorScreenType,
             appName = "Element X",
-            onRetry = {}
+            onRetry = {},
+            onCancel = {},
         )
     }
 }

features/login/impl/src/test/kotlin/io/element/android/features/login/impl/qrcode/QrCodeLoginFlowNodeTest.kt

@@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.auth.qrlogin.QrLoginException
 import io.element.android.libraries.matrix.test.A_SESSION_ID
 import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
 import io.element.android.libraries.matrix.test.auth.qrlogin.FakeMatrixQrCodeLoginData
+import io.element.android.tests.testutils.lambda.lambdaError
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestScope
@@ -183,7 +184,11 @@ class QrCodeLoginFlowNodeTest {
         )
         return QrCodeLoginFlowNode(
             buildContext = buildContext,
-            plugins = emptyList(),
+            plugins = listOf(
+                object : QrCodeLoginFlowNode.Callback {
+                    override fun navigateBack() = lambdaError()
+                }
+            ),
             qrCodeLoginGraphFactory = FakeQrCodeLoginGraph.Builder(qrCodeLoginManager),
             coroutineDispatchers = coroutineDispatchers,
         )

features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorViewTest.kt

@@ -12,8 +12,9 @@ import androidx.activity.ComponentActivity
 import androidx.compose.ui.test.junit4.AndroidComposeTestRule
 import androidx.compose.ui.test.junit4.createAndroidComposeRule
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import io.element.android.features.login.impl.R
 import io.element.android.features.login.impl.qrcode.QrCodeErrorScreenType
+import io.element.android.libraries.ui.strings.CommonStrings
+import io.element.android.tests.testutils.EnsureNeverCalled
 import io.element.android.tests.testutils.clickOn
 import io.element.android.tests.testutils.ensureCalledOnce
 import io.element.android.tests.testutils.pressBackKey
@@ -28,10 +29,10 @@ class QrCodeErrorViewTest {
     val rule = createAndroidComposeRule<ComponentActivity>()
 
     @Test
-    fun `on back pressed - calls the onRetry callback`() {
+    fun `on back pressed - calls the onCancel callback`() {
         ensureCalledOnce { callback ->
             rule.setQrCodeErrorView(
-                onRetry = callback
+                onCancel = callback,
             )
             rule.pressBackKey()
         }
@@ -41,14 +42,25 @@ class QrCodeErrorViewTest {
     fun `on try again button clicked - calls the expected callback`() {
         ensureCalledOnce { callback ->
             rule.setQrCodeErrorView(
-                onRetry = callback
+                onRetry = callback,
             )
-            rule.clickOn(R.string.screen_qr_code_login_start_over_button)
+            rule.clickOn(CommonStrings.action_try_again)
+        }
+    }
+
+    @Test
+    fun `on cancel button clicked - calls the expected callback`() {
+        ensureCalledOnce { callback ->
+            rule.setQrCodeErrorView(
+                onCancel = callback,
+            )
+            rule.clickOn(CommonStrings.action_cancel)
         }
     }
 
     private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setQrCodeErrorView(
-        onRetry: () -> Unit,
+        onRetry: () -> Unit = EnsureNeverCalled(),
+        onCancel: () -> Unit = EnsureNeverCalled(),
         errorScreenType: QrCodeErrorScreenType = QrCodeErrorScreenType.UnknownError,
         appName: String = "Element X",
     ) {
@@ -56,7 +68,8 @@ class QrCodeErrorViewTest {
             QrCodeErrorView(
                 errorScreenType = errorScreenType,
                 appName = appName,
-                onRetry = onRetry
+                onRetry = onRetry,
+                onCancel = onCancel,
             )
         }
     }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt

@@ -1217,7 +1217,7 @@ class MessagesPresenterTest {
     }
 
     @Test
-    fun `present - shows a "history" icon if the room is encrypted and history is shared`() = runTest {
+    fun `present - shows a history icon if the room is encrypted and history is shared`() = runTest {
         val presenter = createMessagesPresenter(
             joinedRoom = FakeJoinedRoom(
                 baseRoom = FakeBaseRoom(