mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 08:15:02 +00:00
Keep the incoming share displayed when the current account is signed out
The incoming share was displayed in the logged in flow of the current session, so it was destroyed when the current session was signed out, for instance from another device. When another session was signed out, the room select screen was kept and displayed an error if this session was selected. To make the behavior consistent: - The incoming share is now displayed by the RootFlowNode, on top of the logged in flow, and does not depend on a session anymore. The latest used session is passed from the RootFlowNode to the room select screen, through the share flow, and is used as its initial session. - When the root of the RootFlowNode backstack changes while an incoming share is displayed, the new root is set below the incoming share, using the new replaceRoot backstack operation, instead of removing all the elements. - The room select screen handles the invalidation of the current session like for any other session. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8,9 +8,15 @@
|
||||
|
||||
package io.element.android.appnav
|
||||
|
||||
import com.bumble.appyx.core.navigation.NavKey
|
||||
import com.bumble.appyx.navmodel.backstack.BackStack
|
||||
import com.bumble.appyx.navmodel.backstack.BackStackElement
|
||||
import com.bumble.appyx.navmodel.backstack.BackStackElements
|
||||
import com.bumble.appyx.navmodel.backstack.operation.BackStackOperation
|
||||
import com.bumble.appyx.navmodel.backstack.operation.NewRoot
|
||||
import com.bumble.appyx.navmodel.backstack.operation.Remove
|
||||
import kotlinx.parcelize.Parcelize
|
||||
import kotlinx.parcelize.RawValue
|
||||
|
||||
/**
|
||||
* Don't process NewRoot if the nav target already exists in the stack.
|
||||
@@ -32,3 +38,45 @@ fun <T : Any> BackStack<T>.removeLast(element: T) {
|
||||
} ?: return
|
||||
accept(Remove(lastExpectedNavElement.key))
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace the root element of the backstack, and keep the elements on top of it, contrary to [safeRoot] which removes all the elements.
|
||||
*/
|
||||
fun <T : Any> BackStack<T>.replaceRoot(element: T) {
|
||||
accept(ReplaceRoot(element))
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace the root element of the backstack, keeping the elements on top of it.
|
||||
*
|
||||
* Operation:
|
||||
*
|
||||
* [A, B, C] + ReplaceRoot(D) = [D, B, C]
|
||||
* [A] + ReplaceRoot(D) = [D]
|
||||
*/
|
||||
@Parcelize
|
||||
internal data class ReplaceRoot<T : Any>(
|
||||
private val element: @RawValue T
|
||||
) : BackStackOperation<T> {
|
||||
override fun isApplicable(elements: BackStackElements<T>): Boolean =
|
||||
elements.rootElement()?.key?.navTarget != element
|
||||
|
||||
override fun invoke(elements: BackStackElements<T>): BackStackElements<T> {
|
||||
val root = elements.rootElement() ?: return elements
|
||||
if (root.targetState == BackStack.State.ACTIVE) {
|
||||
// Nothing on top of the root, so this is the same as NewRoot
|
||||
return NewRoot(element).invoke(elements)
|
||||
}
|
||||
// The root is stashed below other elements: drop it, as Remove does for a non active element,
|
||||
// and add the new root, stashed, in place of it.
|
||||
val newRoot = BackStackElement(
|
||||
key = NavKey(element),
|
||||
fromState = BackStack.State.STASHED,
|
||||
targetState = BackStack.State.STASHED,
|
||||
operation = this,
|
||||
)
|
||||
return elements.map { if (it == root) newRoot else it }
|
||||
}
|
||||
|
||||
private fun BackStackElements<T>.rootElement() = firstOrNull { it.targetState != BackStack.State.DESTROYED }
|
||||
}
|
||||
|
||||
@@ -33,8 +33,6 @@ import io.element.android.libraries.architecture.createNode
|
||||
import io.element.android.libraries.architecture.inputs
|
||||
import io.element.android.libraries.di.DependencyInjectionGraphOwner
|
||||
import io.element.android.libraries.matrix.api.MatrixClient
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.api.core.SessionId
|
||||
import io.element.android.libraries.matrixmedia.api.ImageLoaderHolder
|
||||
import kotlinx.parcelize.Parcelize
|
||||
|
||||
@@ -61,7 +59,6 @@ class LoggedInAppScopeFlowNode(
|
||||
interface Callback : Plugin {
|
||||
fun navigateToBugReport()
|
||||
fun navigateToAddAccount()
|
||||
fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?)
|
||||
}
|
||||
|
||||
private val callback: Callback = callback()
|
||||
@@ -94,10 +91,6 @@ class LoggedInAppScopeFlowNode(
|
||||
override fun navigateToAddAccount() {
|
||||
callback.navigateToAddAccount()
|
||||
}
|
||||
|
||||
override fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) {
|
||||
callback.switchAccountAndOpenRoom(sessionId, roomId)
|
||||
}
|
||||
}
|
||||
return createNode<LoggedInFlowNode>(buildContext, listOf(callback))
|
||||
}
|
||||
|
||||
@@ -67,8 +67,6 @@ import io.element.android.features.preferences.api.PreferencesEntryPoint
|
||||
import io.element.android.features.roomdirectory.api.RoomDescription
|
||||
import io.element.android.features.roomdirectory.api.RoomDirectoryEntryPoint
|
||||
import io.element.android.features.securebackup.api.SecureBackupEntryPoint
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.startchat.api.StartChatEntryPoint
|
||||
import io.element.android.features.userprofile.api.UserProfileEntryPoint
|
||||
import io.element.android.features.verifysession.api.IncomingVerificationEntryPoint
|
||||
@@ -88,7 +86,6 @@ import io.element.android.libraries.matrix.api.MatrixClient
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
|
||||
import io.element.android.libraries.matrix.api.core.SessionId
|
||||
import io.element.android.libraries.matrix.api.core.UserId
|
||||
import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
|
||||
import io.element.android.libraries.matrix.api.permalink.PermalinkData
|
||||
@@ -138,7 +135,6 @@ class LoggedInFlowNode(
|
||||
private val sessionCoroutineScope: CoroutineScope,
|
||||
private val ftueService: FtueService,
|
||||
private val roomDirectoryEntryPoint: RoomDirectoryEntryPoint,
|
||||
private val shareEntryPoint: ShareEntryPoint,
|
||||
private val matrixClient: MatrixClient,
|
||||
private val sendingQueue: SendQueues,
|
||||
private val nativeCallHost: NativeCallHost,
|
||||
@@ -174,7 +170,6 @@ class LoggedInFlowNode(
|
||||
interface Callback : Plugin {
|
||||
fun navigateToBugReport()
|
||||
fun navigateToAddAccount()
|
||||
fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?)
|
||||
}
|
||||
|
||||
private val callback: Callback = callback()
|
||||
@@ -296,9 +291,6 @@ class LoggedInFlowNode(
|
||||
@Parcelize
|
||||
data object RoomDirectory : NavTarget
|
||||
|
||||
@Parcelize
|
||||
data class IncomingShare(val shareIntentData: ShareIntentData) : NavTarget
|
||||
|
||||
@Parcelize
|
||||
data class IncomingVerificationRequest(val data: VerificationRequest.Incoming) : NavTarget
|
||||
}
|
||||
@@ -570,36 +562,6 @@ class LoggedInFlowNode(
|
||||
},
|
||||
)
|
||||
}
|
||||
is NavTarget.IncomingShare -> {
|
||||
shareEntryPoint.createNode(
|
||||
parentNode = this,
|
||||
buildContext = buildContext,
|
||||
params = ShareEntryPoint.Params(shareIntentData = navTarget.shareIntentData),
|
||||
callback = object : ShareEntryPoint.Callback {
|
||||
override fun onDone(sessionId: SessionId, roomIds: List<RoomId>) {
|
||||
// Remove the incoming share screen
|
||||
backstack.pop()
|
||||
|
||||
if (sessionId != matrixClient.sessionId) {
|
||||
// The data has been shared using another session, switch to it
|
||||
callback.switchAccountAndOpenRoom(sessionId, roomIds.singleOrNull())
|
||||
return
|
||||
}
|
||||
|
||||
// Navigate to the room if the text/media was shared to a single one
|
||||
roomIds.singleOrNull()?.let { roomId ->
|
||||
lifecycleScope.launch {
|
||||
// Wait until the incoming share screen is removed
|
||||
backstack.elements.first { it.lastOrNull()?.key?.navTarget !is NavTarget.IncomingShare }
|
||||
|
||||
// Then attach the room
|
||||
attachRoom(roomId.toRoomIdOrAlias(), clearBackstack = false)
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
)
|
||||
}
|
||||
is NavTarget.IncomingVerificationRequest -> {
|
||||
incomingVerificationEntryPoint.createNode(
|
||||
parentNode = this,
|
||||
@@ -659,17 +621,6 @@ class LoggedInFlowNode(
|
||||
}
|
||||
}
|
||||
|
||||
internal suspend fun attachIncomingShare(shareIntentData: ShareIntentData) {
|
||||
waitForNavTargetAttached { navTarget ->
|
||||
navTarget is NavTarget.Home
|
||||
}
|
||||
attachChild<Node> {
|
||||
backstack.push(
|
||||
NavTarget.IncomingShare(shareIntentData)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
override fun View(modifier: Modifier) {
|
||||
val colors by remember {
|
||||
|
||||
@@ -47,6 +47,7 @@ import io.element.android.features.announcement.api.AnnouncementService
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl
|
||||
import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.api.ShareIntentData
|
||||
import io.element.android.features.signedout.api.SignedOutEntryPoint
|
||||
import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint
|
||||
@@ -112,6 +113,7 @@ class RootFlowNode(
|
||||
private val analyticsService: AnalyticsService,
|
||||
private val analyticsColdStartWatcher: AnalyticsColdStartWatcher,
|
||||
private val incomingVerificationRequestObserver: IncomingVerificationRequestObserver,
|
||||
private val shareEntryPoint: ShareEntryPoint,
|
||||
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
|
||||
) : BaseFlowNode<RootFlowNode.NavTarget>(
|
||||
backstack = BackStack(
|
||||
@@ -216,7 +218,7 @@ class RootFlowNode(
|
||||
|
||||
private fun switchToLoggedInFlow(sessionId: SessionId, navId: Int) {
|
||||
pendingLoginParams = null
|
||||
backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, navId))
|
||||
setRoot(NavTarget.LoggedInFlow(sessionId, navId))
|
||||
restoreAllSessions()
|
||||
}
|
||||
|
||||
@@ -286,7 +288,20 @@ class RootFlowNode(
|
||||
|
||||
private fun switchToSignedOutFlow(sessionId: SessionId) {
|
||||
pendingLoginParams = null
|
||||
backstack.safeRoot(NavTarget.SignedOutFlow(sessionId))
|
||||
setRoot(NavTarget.SignedOutFlow(sessionId))
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the root of the backstack. If an incoming share is displayed, keep it on top of the new root, so that the user can
|
||||
* finish sharing, for instance with another account if the current one has been signed out.
|
||||
*/
|
||||
private fun setRoot(navTarget: NavTarget) {
|
||||
val hasIncomingShare = backstack.elements.value.any { it.key.navTarget is NavTarget.IncomingShare }
|
||||
if (hasIncomingShare) {
|
||||
backstack.replaceRoot(navTarget)
|
||||
} else {
|
||||
backstack.safeRoot(navTarget)
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun restoreSessionIfNeeded(
|
||||
@@ -367,6 +382,11 @@ class RootFlowNode(
|
||||
|
||||
@Parcelize data object BugReport : NavTarget
|
||||
|
||||
@Parcelize data class IncomingShare(
|
||||
val sessionId: SessionId,
|
||||
val shareIntentData: ShareIntentData,
|
||||
) : NavTarget
|
||||
|
||||
@Parcelize data class OtherAccountIncomingVerificationRequest(
|
||||
val sessionId: SessionId,
|
||||
val verificationRequest: VerificationRequest.Incoming,
|
||||
@@ -389,15 +409,6 @@ class RootFlowNode(
|
||||
override fun navigateToAddAccount() {
|
||||
backstack.push(NavTarget.NotLoggedInFlow(null))
|
||||
}
|
||||
|
||||
override fun switchAccountAndOpenRoom(sessionId: SessionId, roomId: RoomId?) {
|
||||
lifecycleScope.launch {
|
||||
val loggedInFlowNode = attachSession(sessionId)
|
||||
roomId?.let {
|
||||
loggedInFlowNode.attachRoom(it.toRoomIdOrAlias(), clearBackstack = false)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
val savedNavState = extractSavedStateForNavTarget(navTarget, this.buildContext.savedStateMap)
|
||||
val buildContext = if (savedNavState != null) {
|
||||
@@ -451,6 +462,35 @@ class RootFlowNode(
|
||||
}
|
||||
createNode<OtherSessionIncomingVerificationNode>(buildContext, plugins = listOf(inputs, callback))
|
||||
}
|
||||
is NavTarget.IncomingShare -> {
|
||||
val callback = object : ShareEntryPoint.Callback {
|
||||
override fun onDone(sessionId: SessionId, roomIds: List<RoomId>) {
|
||||
// Remove the incoming share screen
|
||||
backstack.pop()
|
||||
lifecycleScope.launch {
|
||||
// Switch to the session which has been used to share the data, if necessary,
|
||||
// and navigate to the room if the data was shared to a single one.
|
||||
val loggedInFlowNode = attachSession(sessionId)
|
||||
roomIds.singleOrNull()?.let { roomId ->
|
||||
loggedInFlowNode.attachRoom(roomId.toRoomIdOrAlias(), clearBackstack = false)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override fun onCancel() {
|
||||
backstack.pop()
|
||||
}
|
||||
}
|
||||
shareEntryPoint.createNode(
|
||||
parentNode = this,
|
||||
buildContext = buildContext,
|
||||
params = ShareEntryPoint.Params(
|
||||
sessionId = navTarget.sessionId,
|
||||
shareIntentData = navTarget.shareIntentData,
|
||||
),
|
||||
callback = callback,
|
||||
)
|
||||
}
|
||||
NavTarget.BugReport -> {
|
||||
val callback = object : BugReportEntryPoint.Callback {
|
||||
override fun onDone() {
|
||||
@@ -542,9 +582,11 @@ class RootFlowNode(
|
||||
// No session, open login
|
||||
switchToNotLoggedInFlow(null)
|
||||
} else {
|
||||
// wait for the current session to be restored, then attach the incoming share node.
|
||||
// In case of multiple accounts, the user can switch account from the room select screen.
|
||||
attachSession(latestSessionId).attachIncomingShare(shareIntentData)
|
||||
// Wait for the current session to be restored, then display the incoming share on top of it.
|
||||
// The incoming share does not belong to the current session: in case of multiple accounts, the user can switch account
|
||||
// from the room select screen, and the share is kept if the current session is signed out in the meantime.
|
||||
attachSession(latestSessionId)
|
||||
backstack.push(NavTarget.IncomingShare(latestSessionId, shareIntentData))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.appnav
|
||||
|
||||
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.google.common.truth.Truth.assertThat
|
||||
import org.junit.Test
|
||||
|
||||
class BackstackExtTest {
|
||||
@Test
|
||||
fun `replaceRoot without element on top replaces the root`() {
|
||||
val backStack = BackStack(initialElement = "A", savedStateMap = null)
|
||||
backStack.replaceRoot("D")
|
||||
assertThat(backStack.navTargets()).containsExactly("D")
|
||||
assertThat(backStack.activeTarget()).isEqualTo("D")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaceRoot with elements on top replaces the root and keeps the elements on top`() {
|
||||
val backStack = BackStack(initialElement = "A", savedStateMap = null)
|
||||
backStack.push("B")
|
||||
backStack.push("C")
|
||||
backStack.replaceRoot("D")
|
||||
assertThat(backStack.navTargets()).containsExactly("D", "B", "C").inOrder()
|
||||
assertThat(backStack.activeTarget()).isEqualTo("C")
|
||||
// When the elements on top are removed, the new root is displayed
|
||||
backStack.pop()
|
||||
backStack.pop()
|
||||
assertThat(backStack.activeTarget()).isEqualTo("D")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaceRoot with the current root does nothing`() {
|
||||
val backStack = BackStack(initialElement = "A", savedStateMap = null)
|
||||
backStack.push("B")
|
||||
backStack.replaceRoot("A")
|
||||
assertThat(backStack.navTargets()).containsExactly("A", "B").inOrder()
|
||||
assertThat(backStack.activeTarget()).isEqualTo("B")
|
||||
}
|
||||
|
||||
private fun BackStack<String>.navTargets() = elements.value
|
||||
.filter { it.targetState != BackStack.State.DESTROYED }
|
||||
.map { it.key.navTarget }
|
||||
|
||||
private fun BackStack<String>.activeTarget() = elements.value
|
||||
.single { it.targetState == BackStack.State.ACTIVE }
|
||||
.key
|
||||
.navTarget
|
||||
}
|
||||
@@ -14,6 +14,7 @@ import android.net.Uri
|
||||
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
|
||||
import com.bumble.appyx.core.modality.AncestryInfo
|
||||
import com.bumble.appyx.core.modality.BuildContext
|
||||
import com.bumble.appyx.core.node.Node
|
||||
import com.bumble.appyx.core.node.node
|
||||
import com.bumble.appyx.navmodel.backstack.activeElement
|
||||
import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
|
||||
@@ -34,6 +35,7 @@ import io.element.android.features.login.test.FakeLoginIntentResolver
|
||||
import io.element.android.features.login.test.accesscontrol.FakeAccountProviderAccessControl
|
||||
import io.element.android.features.preferences.test.FakeCacheService
|
||||
import io.element.android.features.rageshake.test.FakeBugReportEntryPoint
|
||||
import io.element.android.features.share.api.ShareEntryPoint
|
||||
import io.element.android.features.share.test.FakeShareIntentHandler
|
||||
import io.element.android.features.signedout.test.FakeSignedOutEntryPoint
|
||||
import io.element.android.libraries.accountselect.test.FakeAccountSelectEntryPoint
|
||||
@@ -52,6 +54,7 @@ import io.element.android.services.analytics.test.FakeAnalyticsService
|
||||
import io.element.android.services.analytics.test.watchers.FakeAnalyticsColdStartWatcher
|
||||
import io.element.android.services.apperror.test.FakeAppErrorStateService
|
||||
import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
|
||||
import io.element.android.tests.testutils.lambda.lambdaError
|
||||
import io.element.android.tests.testutils.node.FakeNodeFactoriesBindings
|
||||
import io.element.android.tests.testutils.node.FakeParentNode
|
||||
import io.element.android.tests.testutils.presenter.NotUsedPresenter
|
||||
@@ -192,6 +195,14 @@ class RootFlowNodeTest : RobolectricTest() {
|
||||
appForegroundStateService = FakeAppForegroundStateService(),
|
||||
),
|
||||
appCoroutineScope = backgroundScope,
|
||||
shareEntryPoint = object : ShareEntryPoint {
|
||||
override fun createNode(
|
||||
parentNode: Node,
|
||||
buildContext: BuildContext,
|
||||
params: ShareEntryPoint.Params,
|
||||
callback: ShareEntryPoint.Callback,
|
||||
): Node = lambdaError()
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user