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

tom
2026-01-28 16:09:31 +01:00
parent 692302df5c
commit 36d5500a65
10 changed files with 120 additions and 28 deletions
--- a/features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/LinkNewDeviceFlowNode.kt
+++ b/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))
            }
--- a/features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorNode.kt
+++ b/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,
        )
    }
 }
--- a/features/linknewdevice/impl/src/main/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorView.kt
+++ b/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 = {},
        )
    }
 }
--- a/features/linknewdevice/impl/src/test/kotlin/io/element/android/features/linknewdevice/impl/screens/error/ErrorViewTest.kt
+++ b/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,
            )
        }
    }
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt
+++ b/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(
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/qrcode/QrCodeLoginFlowNode.kt
+++ b/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))
            }
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorNode.kt
+++ b/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,
        )
    }
 }
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorView.kt
+++ b/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 = {},
        )
    }
 }
--- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/qrcode/QrCodeLoginFlowNodeTest.kt
+++ b/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,
        )
--- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/error/QrCodeErrorViewTest.kt
+++ b/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,
            )
        }
    }