From 528e7b390b12e7dfcb44b15064a50b9be0232671 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 May 2024 12:58:17 +0200 Subject: [PATCH 1/5] Remove the FtueEntryPoint.Callback, LoggedInFlowNode is already observing the Ftue state to change the root target. --- .../kotlin/io/element/android/appnav/LoggedInFlowNode.kt | 5 ----- .../io/element/android/features/ftue/api/FtueEntryPoint.kt | 6 ------ .../android/features/ftue/impl/DefaultFtueEntryPoint.kt | 5 ----- .../io/element/android/features/ftue/impl/FtueFlowNode.kt | 5 +---- 4 files changed, 1 insertion(+), 20 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index ecff658963..5ba37a79d6 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -345,11 +345,6 @@ class LoggedInFlowNode @AssistedInject constructor( } NavTarget.Ftue -> { ftueEntryPoint.nodeBuilder(this, buildContext) - .callback(object : FtueEntryPoint.Callback { - override fun onFtueFlowFinished() { - lifecycleScope.launch { attachRoomList() } - } - }) .build() } NavTarget.RoomDirectorySearch -> { diff --git a/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/FtueEntryPoint.kt b/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/FtueEntryPoint.kt index 186f2fe026..f3ad0b64a9 100644 --- a/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/FtueEntryPoint.kt +++ b/features/ftue/api/src/main/kotlin/io/element/android/features/ftue/api/FtueEntryPoint.kt @@ -18,18 +18,12 @@ package io.element.android.features.ftue.api import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node -import com.bumble.appyx.core.plugin.Plugin import io.element.android.libraries.architecture.FeatureEntryPoint interface FtueEntryPoint : FeatureEntryPoint { fun nodeBuilder(parentNode: Node, buildContext: BuildContext): NodeBuilder interface NodeBuilder { - fun callback(callback: Callback): NodeBuilder fun build(): Node } - - interface Callback : Plugin { - fun onFtueFlowFinished() - } } diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPoint.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPoint.kt index c89b1840af..0b089c3df9 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPoint.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/DefaultFtueEntryPoint.kt @@ -31,11 +31,6 @@ class DefaultFtueEntryPoint @Inject constructor() : FtueEntryPoint { val plugins = ArrayList() return object : FtueEntryPoint.NodeBuilder { - override fun callback(callback: FtueEntryPoint.Callback): FtueEntryPoint.NodeBuilder { - plugins += callback - return this - } - override fun build(): Node { return parentNode.createNode(buildContext, plugins) } diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt index 3616faf2ae..772343ea58 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt @@ -32,7 +32,6 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.features.analytics.api.AnalyticsEntryPoint -import io.element.android.features.ftue.api.FtueEntryPoint import io.element.android.features.ftue.impl.notifications.NotificationsOptInNode import io.element.android.features.ftue.impl.sessionverification.FtueSessionVerificationFlowNode import io.element.android.features.ftue.impl.state.DefaultFtueService @@ -86,8 +85,6 @@ class FtueFlowNode @AssistedInject constructor( data object LockScreenSetup : NavTarget } - private val callback = plugins.filterIsInstance().firstOrNull() - override fun onBuilt() { super.onBuilt() @@ -157,7 +154,7 @@ class FtueFlowNode @AssistedInject constructor( FtueStep.LockscreenSetup -> { backstack.newRoot(NavTarget.LockScreenSetup) } - null -> callback?.onFtueFlowFinished() + null -> Unit } } From b05fa82dfc36a089c9ca1ef0252bdeb8be0bdf3b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 May 2024 12:47:48 +0200 Subject: [PATCH 2/5] Rename val for clarity. --- .../main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index 5ba37a79d6..9d176321ca 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -386,9 +386,9 @@ class LoggedInFlowNode @AssistedInject constructor( override fun View(modifier: Modifier) { Box(modifier = modifier) { val lockScreenState by lockScreenStateService.lockState.collectAsState() - val isFtueDisplayed by ftueService.state.collectAsState() + val ftueState by ftueService.state.collectAsState() BackstackView() - if (isFtueDisplayed is FtueState.Complete) { + if (ftueState is FtueState.Complete) { PermanentChild(permanentNavModel = permanentNavModel, navTarget = NavTarget.LoggedInPermanent) } if (lockScreenState == LockScreenLockState.Locked) { From 83506e51913899a775475214f63f3bee0ec059df Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 May 2024 13:50:34 +0200 Subject: [PATCH 3/5] Fix navigation issue #2778 --- .../android/appnav/LoggedInFlowNode.kt | 18 ++++------------ .../io/element/android/appnav/RootFlowNode.kt | 2 +- .../libraries/architecture/ParentNodeExt.kt | 21 +++++++++++++++++++ 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index 9d176321ca..fed9649c85 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -35,7 +35,6 @@ import com.bumble.appyx.core.plugin.plugins import com.bumble.appyx.navmodel.backstack.BackStack import com.bumble.appyx.navmodel.backstack.operation.push import com.bumble.appyx.navmodel.backstack.operation.replace -import com.bumble.appyx.navmodel.backstack.operation.singleTop import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode @@ -60,6 +59,7 @@ import io.element.android.features.securebackup.api.SecureBackupEntryPoint import io.element.android.libraries.architecture.BackstackView import io.element.android.libraries.architecture.BaseFlowNode import io.element.android.libraries.architecture.createNode +import io.element.android.libraries.architecture.waitForNavTargetAttached import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher import io.element.android.libraries.di.AppScope import io.element.android.libraries.di.SessionScope @@ -363,25 +363,15 @@ class LoggedInFlowNode @AssistedInject constructor( } } - suspend fun attachRoomList() { - if (!canShowRoomList()) return - attachChild { - backstack.singleTop(NavTarget.RoomList) - } - } - suspend fun attachRoom(roomId: RoomId) { - if (!canShowRoomList()) return + waitForNavTargetAttached { navTarget -> + navTarget is NavTarget.RoomList + } attachChild { - backstack.singleTop(NavTarget.RoomList) backstack.push(NavTarget.Room(roomId.toRoomIdOrAlias())) } } - private fun canShowRoomList(): Boolean { - return ftueService.state.value is FtueState.Complete - } - @Composable override fun View(modifier: Modifier) { Box(modifier = modifier) { diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index a3cbfb7d77..7f06f2833b 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -288,7 +288,7 @@ class RootFlowNode @AssistedInject constructor( .attachSession() .apply { when (deeplinkData) { - is DeeplinkData.Root -> attachRoomList() + is DeeplinkData.Root -> Unit // The room list will always be shown, observing FtueState is DeeplinkData.Room -> attachRoom(deeplinkData.roomId) } } diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt index c03ff64f6c..4ddd8c78a4 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt @@ -20,6 +20,7 @@ import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.children.nodeOrNull import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode +import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import kotlinx.coroutines.suspendCancellableCoroutine import kotlin.coroutines.resume @@ -48,3 +49,23 @@ suspend inline fun ParentNode.wai continuation.cancel() } } + +/** + * Wait for a child to be attached to the parent node, only using the NavTarget + */ +suspend inline fun ParentNode.waitForNavTargetAttached(crossinline predicate: (NavTarget) -> Boolean) = + suspendCancellableCoroutine { continuation -> + lifecycleScope.launch { + children.collect { childMap -> + val node = childMap.entries + .map { it.key.navTarget } + .lastOrNull(predicate) + if (node != null && !continuation.isCompleted) { + continuation.resume(Unit) + cancel() + } + } + }.invokeOnCompletion { + continuation.cancel() + } + } From 17913878b1150425dc84d9c3550d285839ecf7d4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 May 2024 14:02:10 +0200 Subject: [PATCH 4/5] Changelog --- changelog.d/2778.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/2778.bugfix diff --git a/changelog.d/2778.bugfix b/changelog.d/2778.bugfix new file mode 100644 index 0000000000..9353dda39f --- /dev/null +++ b/changelog.d/2778.bugfix @@ -0,0 +1 @@ +Ensure the application open the room when a notification is clicked. From f807d578fabb493ab70f1ac40a1cc6bc5e690c15 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 1 May 2024 19:35:16 +0200 Subject: [PATCH 5/5] Period --- .../io/element/android/libraries/architecture/ParentNodeExt.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt index 4ddd8c78a4..be68c4671e 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt @@ -51,7 +51,7 @@ suspend inline fun ParentNode.wai } /** - * Wait for a child to be attached to the parent node, only using the NavTarget + * Wait for a child to be attached to the parent node, only using the NavTarget. */ suspend inline fun ParentNode.waitForNavTargetAttached(crossinline predicate: (NavTarget) -> Boolean) = suspendCancellableCoroutine { continuation ->