From b82daa55d68d9ee655d5adf4b19be7ed213b353b Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Mon, 2 Oct 2023 08:41:24 +0100 Subject: [PATCH 1/4] Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode. --- .../appyx/core/node/PermanentChildTest.kt | 35 +++++-- .../appyx/core/composable/PermanentChild.kt | 56 +++++++++++ .../com/bumble/appyx/core/node/ParentNode.kt | 70 +------------ .../bumble/appyx/core/node/ParentNodeTest.kt | 98 +++++++++++++++++-- .../app/node/samples/SamplesSelectorNode.kt | 21 +++- .../node/slideshow/slide/app/StatefulNode1.kt | 27 +++-- .../client/combined/CombinedNavModelNode.kt | 12 ++- 7 files changed, 219 insertions(+), 100 deletions(-) create mode 100644 libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt diff --git a/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt b/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt index 3298e7286..b9997e791 100644 --- a/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt +++ b/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt @@ -11,8 +11,9 @@ import androidx.compose.ui.platform.testTag import androidx.compose.ui.test.hasTestTag import com.bumble.appyx.core.AppyxTestScenario import com.bumble.appyx.core.children.nodeOrNull +import com.bumble.appyx.core.composable.PermanentChild import com.bumble.appyx.core.modality.BuildContext -import com.bumble.appyx.core.navigation.EmptyNavModel +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import kotlinx.parcelize.Parcelize import org.junit.Assert.assertEquals import org.junit.Rule @@ -20,20 +21,26 @@ import org.junit.Test class PermanentChildTest { + var nodeFactory: (buildContext: BuildContext) -> TestParentNode = { + TestParentNode(buildContext = it) + } + @get:Rule val rule = AppyxTestScenario { buildContext -> - TestParentNode(buildContext) + nodeFactory(buildContext) } @Test - fun permanent_child_is_rendered() { + fun `WHEN_permanent_model_contains_relevant_nav_key_THEN_permanent_child_is_rendered`() { + createPermanentNavModelWithNavKey() rule.start() rule.onNode(hasTestTag(TestParentNode.NavTarget::class.java.name)).assertExists() } @Test - fun permanent_child_is_reused_when_visibility_switched() { + fun `WHEN_visibility_switched_THEN_permanent_child_is_reused`() { + createPermanentNavModelWithNavKey() rule.start() rule.node.renderPermanentChild = false val childNodes = rule.node.children.value.values.map { it.nodeOrNull } @@ -46,11 +53,27 @@ class PermanentChildTest { assertEquals(childNodes, rule.node.children.value.values.map { it.nodeOrNull }) } + private fun createPermanentNavModelWithNavKey() { + nodeFactory = { + TestParentNode( + buildContext = it, + permanentNavModel = PermanentNavModel( + TestParentNode.NavTarget, + savedStateMap = null, + ) + ) + } + + } + class TestParentNode( buildContext: BuildContext, + private val permanentNavModel: PermanentNavModel = PermanentNavModel( + savedStateMap = buildContext.savedStateMap + ), ) : ParentNode( buildContext = buildContext, - navModel = EmptyNavModel(), + navModel = permanentNavModel ) { @Parcelize @@ -69,7 +92,7 @@ class PermanentChildTest { @Composable override fun View(modifier: Modifier) { if (renderPermanentChild) { - PermanentChild(NavTarget) + PermanentChild(permanentNavModel, NavTarget) } } } diff --git a/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt new file mode 100644 index 000000000..28337c4e2 --- /dev/null +++ b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt @@ -0,0 +1,56 @@ +package com.bumble.appyx.core.composable + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember +import androidx.compose.ui.Modifier +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.node.ParentNode + +@Composable +fun ParentNode.PermanentChild( + permanentNavModel: PermanentNavModel, + navTarget: NavTarget, + decorator: @Composable (child: ChildRenderer) -> Unit +) { + val child = remember(navTarget) { + permanentNavModel + .elements + .value + .find { it.key.navTarget == navTarget } + ?.let { childOrCreate(it.key) } + ?: throw IllegalStateException( + "No child found for $navTarget in $permanentNavModel. " + + "Add $navTarget to $permanentNavModel before calling PermanentChild." + ) + } + + decorator(PermanentChildRender(child.node)) +} + +@Composable +fun ParentNode.PermanentChild( + permanentNavModel: PermanentNavModel, + navTarget: NavTarget, +) { + PermanentChild(permanentNavModel, navTarget) { child -> child() } +} + +private class PermanentChildRender(private val node: Node) : ChildRenderer { + + @Suppress( + "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'. + ) + @Composable + override operator fun invoke(modifier: Modifier) { + node.Compose(modifier) + } + + @Suppress( + "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'. + ) + @Composable + override operator fun invoke() { + invoke(modifier = Modifier) + } +} diff --git a/libraries/core/src/main/kotlin/com/bumble/appyx/core/node/ParentNode.kt b/libraries/core/src/main/kotlin/com/bumble/appyx/core/node/ParentNode.kt index 2ae5b270d..866cc4a5a 100644 --- a/libraries/core/src/main/kotlin/com/bumble/appyx/core/node/ParentNode.kt +++ b/libraries/core/src/main/kotlin/com/bumble/appyx/core/node/ParentNode.kt @@ -2,14 +2,7 @@ package com.bumble.appyx.core.node import androidx.annotation.CallSuper import androidx.annotation.VisibleForTesting -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Stable -import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.ui.Modifier import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner @@ -24,21 +17,15 @@ import com.bumble.appyx.core.children.ChildEntryMap import com.bumble.appyx.core.children.ChildNodeCreationManager import com.bumble.appyx.core.children.ChildrenCallback import com.bumble.appyx.core.children.nodeOrNull -import com.bumble.appyx.core.composable.ChildRenderer import com.bumble.appyx.core.lifecycle.ChildNodeLifecycleManager -import com.bumble.appyx.core.mapState import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.navigation.NavKey import com.bumble.appyx.core.navigation.NavModel import com.bumble.appyx.core.navigation.Resolver import com.bumble.appyx.core.navigation.isTransitioning -import com.bumble.appyx.core.navigation.model.combined.plus -import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel -import com.bumble.appyx.core.navigation.model.permanent.operation.addUnique import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.core.state.MutableSavedStateMap import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.suspendCancellableCoroutine @@ -50,7 +37,7 @@ import kotlin.reflect.KClass @Suppress("TooManyFunctions") @Stable abstract class ParentNode( - navModel: NavModel, + val navModel: NavModel, buildContext: BuildContext, view: ParentNodeView = EmptyParentNodeView(), childKeepMode: ChildEntry.KeepMode = Appyx.defaultChildKeepMode, @@ -62,12 +49,6 @@ abstract class ParentNode( plugins = plugins + navModel + childAware ), Resolver { - private val permanentNavModel = PermanentNavModel( - savedStateMap = buildContext.savedStateMap, - key = KEY_PERMANENT_NAV_MODEL, - ) - val navModel: NavModel = permanentNavModel + navModel - private val childNodeCreationManager = ChildNodeCreationManager( savedStateMap = buildContext.savedStateMap, customisations = buildContext.customisations, @@ -96,35 +77,6 @@ abstract class ParentNode( fun childOrCreate(navKey: NavKey): ChildEntry.Initialized = childNodeCreationManager.childOrCreate(navKey) - @Composable - fun PermanentChild( - navTarget: NavTarget, - decorator: @Composable (child: ChildRenderer) -> Unit - ) { - LaunchedEffect(navTarget) { - permanentNavModel.addUnique(navTarget) - } - val scope = rememberCoroutineScope() - val child by remember(navTarget) { - permanentNavModel - .elements - // use WhileSubscribed or Lazy otherwise desynchronisation issue - .mapState(scope, SharingStarted.WhileSubscribed()) { navElements -> - navElements - .find { it.key.navTarget == navTarget } - ?.let { childOrCreate(it.key) } - } - }.collectAsState() - child?.let { - decorator(child = PermanentChildRender(it.node)) - } - } - - @Composable - fun PermanentChild(navTarget: NavTarget) { - PermanentChild(navTarget) { child -> child() } - } - override fun updateLifecycleState(state: Lifecycle.State) { super.updateLifecycleState(state) childNodeLifecycleManager.propagateLifecycleToChildren(state) @@ -225,8 +177,6 @@ abstract class ParentNode( @CallSuper override fun onSaveInstanceState(state: MutableSavedStateMap) { super.onSaveInstanceState(state) - // permanentNavModel is not provided as a plugin, store manually - permanentNavModel.saveInstanceState(state) childNodeCreationManager.saveChildrenState(state) } @@ -266,26 +216,8 @@ abstract class ParentNode( companion object { const val ATTACH_WORKFLOW_SYNC_TIMEOUT = 5000L - const val KEY_PERMANENT_NAV_MODEL = "PermanentNavModel" } - private class PermanentChildRender(private val node: Node) : ChildRenderer { - - @Suppress( - "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'. - ) - @Composable - override operator fun invoke(modifier: Modifier) { - node.Compose(modifier) - } - @Suppress( - "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'. - ) - @Composable - override operator fun invoke() { - invoke(modifier = Modifier) - } - } } diff --git a/libraries/core/src/test/kotlin/com/bumble/appyx/core/node/ParentNodeTest.kt b/libraries/core/src/test/kotlin/com/bumble/appyx/core/node/ParentNodeTest.kt index edd433750..c73a7d4b2 100644 --- a/libraries/core/src/test/kotlin/com/bumble/appyx/core/node/ParentNodeTest.kt +++ b/libraries/core/src/test/kotlin/com/bumble/appyx/core/node/ParentNodeTest.kt @@ -1,11 +1,16 @@ package com.bumble.appyx.core.node import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import com.bumble.appyx.core.children.nodeOrNull import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.navigation.NavModel +import com.bumble.appyx.core.navigation.model.combined.plus +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel +import com.bumble.appyx.core.navigation.model.permanent.operation.addUnique +import com.bumble.appyx.core.node.ParentNodeTest.NavTarget.ChildA +import com.bumble.appyx.core.node.ParentNodeTest.NavTarget.ChildB import com.bumble.appyx.core.node.ParentNodeTest.NodeB.Companion.StatusExecuted -import com.bumble.appyx.core.node.ParentNodeTest.TestParentNode.NavTarget -import com.bumble.appyx.core.node.ParentNodeTest.TestParentNode.NavTarget.ChildA -import com.bumble.appyx.core.node.ParentNodeTest.TestParentNode.NavTarget.ChildB +import com.bumble.appyx.core.state.SavedStateMap import com.bumble.appyx.navmodel.backstack.BackStack import com.bumble.appyx.navmodel.backstack.operation.push import com.bumble.appyx.testing.junit4.util.MainDispatcherRule @@ -75,12 +80,85 @@ class ParentNodeTest { assertTrue(attachedNode is NodeB) } - private fun buildBackStack(initialElement: NavTarget = ChildA) = - BackStack(initialElement = initialElement, savedStateMap = null) + @Test + fun `GIVEN node with PermanentNavModel WHEN saves state THEN restores state correctly`() = + testScope.runTest { + //given + val permanentNavModel: PermanentNavModel = + PermanentNavModel(ChildA, savedStateMap = null) + val node = buildParentNode(navModel = permanentNavModel) + assertChildrenCount(node, 1) + + // when + permanentNavModel.addUnique(ChildB) + assertChildrenCount(node, 2) + val state = node.saveInstanceState { true } + val restoredStateNode = + buildParentNode(navModel = permanentNavModel, savedStateMap = state) + + //then + assertChildrenCount(restoredStateNode, 2) + } + + @Test + fun `GIVEN node with CombinedNavModel with PermanentNavModel WHEN saves state THEN restores state correctly`() = + testScope.runTest { + //given + val permanentNavModel: PermanentNavModel = + PermanentNavModel(ChildA, savedStateMap = null) + + val backStack = buildBackStack(initialElement = ChildB) + val node = buildParentNode(navModel = permanentNavModel + backStack) + assertChildrenCount(node, 2) + + // when + permanentNavModel.addUnique(ChildB) + assertChildrenCount(node, 3) + val state = node.saveInstanceState { true } + val restoredStateNode = + buildParentNode(navModel = permanentNavModel + backStack, savedStateMap = state) + + //then + assertChildrenCount(restoredStateNode, 3) + } + + private fun assertChildrenCount(node: ParentNode<*>, expectedCount: Int) { + val childrenCount = node.children.value.values.mapNotNull { it.nodeOrNull }.count() + assertTrue(childrenCount == expectedCount) + } + + private fun buildBackStack( + initialElement: NavTarget = ChildA, + savedStateMap: SavedStateMap? = null + ) = + BackStack(initialElement = initialElement, savedStateMap = savedStateMap) private fun buildParentNode(backStack: BackStack) = TestParentNode(backStack).apply { onBuilt() } + private fun buildParentNode( + savedStateMap: SavedStateMap? = null, + navModel: NavModel + ) = + TestPermanentModelParentNode( + savedStateMap = savedStateMap, + navModel = navModel + ).apply { onBuilt() } + + private class TestPermanentModelParentNode( + savedStateMap: SavedStateMap? = null, + navModel: NavModel + ) : ParentNode( + buildContext = BuildContext.root(savedStateMap), + navModel = navModel + ) { + + override fun resolve(navTarget: NavTarget, buildContext: BuildContext) = when (navTarget) { + ChildA -> node(buildContext) {} + ChildB -> NodeB(buildContext) + } + } + private class TestParentNode( private val backStack: BackStack ) : ParentNode( @@ -88,11 +166,6 @@ class ParentNodeTest { navModel = backStack ) { - sealed class NavTarget { - object ChildA : NavTarget() - object ChildB : NavTarget() - } - suspend fun waitForBAttached(): NodeB { return waitForChildAttached() } @@ -115,6 +188,11 @@ class ParentNodeTest { } } + private sealed class NavTarget { + object ChildA : NavTarget() + object ChildB : NavTarget() + } + private class NodeB(buildContext: BuildContext) : Node(buildContext) { var status: String? = null diff --git a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/samples/SamplesSelectorNode.kt b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/samples/SamplesSelectorNode.kt index 1870e1ad3..89b4f50ad 100644 --- a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/samples/SamplesSelectorNode.kt +++ b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/samples/SamplesSelectorNode.kt @@ -19,9 +19,10 @@ import com.bumble.appyx.app.node.backstack.InsideTheBackStack import com.bumble.appyx.app.node.cards.CardsExampleNode import com.bumble.appyx.app.node.slideshow.WhatsAppyxSlideShow import com.bumble.appyx.core.composable.ChildRenderer +import com.bumble.appyx.core.composable.PermanentChild import com.bumble.appyx.core.integrationpoint.LocalIntegrationPoint import com.bumble.appyx.core.modality.BuildContext -import com.bumble.appyx.core.navigation.EmptyNavModel +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode import com.bumble.appyx.core.node.node @@ -30,9 +31,16 @@ import kotlinx.parcelize.Parcelize class SamplesSelectorNode( buildContext: BuildContext, + private val permanentNavModel: PermanentNavModel = PermanentNavModel( + NavTarget.InsideTheBackStack, + NavTarget.CardsExample, + NavTarget.OnboardingScreen, + NavTarget.ComposeNavigationScreen, + savedStateMap = buildContext.savedStateMap, + ), private val outputFunc: (Output) -> Unit ) : ParentNode( - navModel = EmptyNavModel(), + navModel = permanentNavModel, buildContext = buildContext ) { sealed class NavTarget : Parcelable { @@ -53,7 +61,7 @@ class SamplesSelectorNode( object OpenCardsExample : Output() object OpenOnboarding : Output() object OpenComposeNavigation : Output() - object OpenInsideTheBackStack: Output() + object OpenInsideTheBackStack : Output() } @ExperimentalUnitApi @@ -67,6 +75,7 @@ class SamplesSelectorNode( buildContext = buildContext, autoAdvanceDelayMs = 2500 ) + is NavTarget.ComposeNavigationScreen -> { node(buildContext) { // compose-navigation fetches the integration point via LocalIntegrationPoint @@ -77,6 +86,7 @@ class SamplesSelectorNode( } } } + is NavTarget.InsideTheBackStack -> InsideTheBackStack( buildContext = buildContext, autoAdvanceDelayMs = 1000 @@ -124,6 +134,7 @@ class SamplesSelectorNode( onClick = { outputFunc(Output.OpenInsideTheBackStack) }, ) { PermanentChild( + permanentNavModel = permanentNavModel, navTarget = NavTarget.InsideTheBackStack, decorator = decorator ) @@ -141,6 +152,7 @@ class SamplesSelectorNode( onClick = { outputFunc(Output.OpenComposeNavigation) }, ) { PermanentChild( + permanentNavModel = permanentNavModel, navTarget = NavTarget.ComposeNavigationScreen, decorator = decorator ) @@ -158,6 +170,7 @@ class SamplesSelectorNode( onClick = { outputFunc(Output.OpenOnboarding) }, ) { PermanentChild( + permanentNavModel = permanentNavModel, navTarget = NavTarget.OnboardingScreen, decorator = decorator ) @@ -172,9 +185,11 @@ class SamplesSelectorNode( onClick = { outputFunc(Output.OpenCardsExample) }, ) { PermanentChild( + permanentNavModel = permanentNavModel, navTarget = NavTarget.CardsExample, decorator = decorator ) + } } } diff --git a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt index 2e8337503..913f3bbe6 100644 --- a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt +++ b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt @@ -25,13 +25,14 @@ import androidx.compose.ui.unit.dp import com.bumble.appyx.app.composable.Page import com.bumble.appyx.app.node.child.GenericChildNode import com.bumble.appyx.app.ui.AppyxSampleAppTheme +import com.bumble.appyx.core.composable.PermanentChild import com.bumble.appyx.core.integration.NodeHost import com.bumble.appyx.core.integrationpoint.IntegrationPointStub import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.modality.BuildContext.Companion.root +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode -import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import kotlinx.coroutines.delay import kotlinx.parcelize.Parcelize @@ -39,12 +40,13 @@ import kotlinx.parcelize.Parcelize @ExperimentalAnimationApi @ExperimentalComposeUiApi class StatefulNode1( - buildContext: BuildContext -) : ParentNode( - buildContext = buildContext, - navModel = PermanentNavModel( + buildContext: BuildContext, + private val permanentNavModel: PermanentNavModel = PermanentNavModel( savedStateMap = buildContext.savedStateMap ) +) : ParentNode( + buildContext = buildContext, + navModel = permanentNavModel, ) { sealed class NavTarget : Parcelable { @Parcelize @@ -62,8 +64,8 @@ class StatefulNode1( modifier = modifier, title = "Stateful", body = "Each Node on this screen has some state:" + - "\n\n1. The counter represents data from a background process (e.g. server)." + - "\n2. You can also tap them to change their colour. Try it!" + "\n\n1. The counter represents data from a background process (e.g. server)." + + "\n2. You can also tap them to change their colour. Try it!" ) { Column(Modifier.fillMaxSize()) { Row( @@ -108,8 +110,15 @@ class StatefulNode1( } @Composable - private fun ChildInABox(navTarget: NavTarget, showWithDelay: Long, modifier: Modifier = Modifier) { - PermanentChild(navTarget) { child -> + private fun ChildInABox( + navTarget: NavTarget, + showWithDelay: Long, + modifier: Modifier = Modifier + ) { + PermanentChild( + permanentNavModel = permanentNavModel, + navTarget + ) { child -> Box(modifier) { var visible by remember { mutableStateOf(false) } AnimatedVisibility( diff --git a/samples/sandbox/src/main/kotlin/com/bumble/appyx/sandbox/client/combined/CombinedNavModelNode.kt b/samples/sandbox/src/main/kotlin/com/bumble/appyx/sandbox/client/combined/CombinedNavModelNode.kt index bc940a8c6..e1f402edd 100644 --- a/samples/sandbox/src/main/kotlin/com/bumble/appyx/sandbox/client/combined/CombinedNavModelNode.kt +++ b/samples/sandbox/src/main/kotlin/com/bumble/appyx/sandbox/client/combined/CombinedNavModelNode.kt @@ -22,13 +22,15 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp import com.bumble.appyx.core.composable.Children +import com.bumble.appyx.core.composable.PermanentChild import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.navigation.model.combined.plus +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode import com.bumble.appyx.navmodel.backstack.BackStack import com.bumble.appyx.navmodel.backstack.operation.push import com.bumble.appyx.navmodel.backstack.transitionhandler.rememberBackstackFader -import com.bumble.appyx.core.navigation.model.combined.plus import com.bumble.appyx.sandbox.client.child.ChildNode import com.bumble.appyx.sandbox.client.combined.CombinedNavModelNode.NavTarget.Dynamic.Child import kotlinx.parcelize.Parcelize @@ -46,9 +48,13 @@ class CombinedNavModelNode( savedStateMap = buildContext.savedStateMap, key = "BackStack2", ), + private val permanentNavModel: PermanentNavModel = PermanentNavModel( + NavTarget.Permanent.Child1, + savedStateMap = buildContext.savedStateMap, + ) ) : ParentNode( buildContext = buildContext, - navModel = backStack1 + backStack2, + navModel = backStack1 + backStack2 + permanentNavModel, ) { sealed class NavTarget : Parcelable { @@ -100,7 +106,7 @@ class CombinedNavModelNode( Button(onClick = { visibility = !visibility }) { Text(text = "Trigger visibility") } - PermanentChild(NavTarget.Permanent.Child1) { child -> + PermanentChild(permanentNavModel, NavTarget.Permanent.Child1) { child -> Box( modifier = Modifier .fillMaxWidth() From 66ad121ed6add4a840fec6447be7de89255e7e58 Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Mon, 2 Oct 2023 08:49:02 +0100 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1866672dd..6a956fcef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Pending changes -- +- [#606](https://github.com/bumble-tech/appyx/pull/606) – **Fixed**: Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode --- From c9de40517c3915aff518b563ad22a1266342c159 Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Mon, 2 Oct 2023 10:25:55 +0100 Subject: [PATCH 3/4] Remember permanentNavModel and add fix StatefulNode1 --- .../kotlin/com/bumble/appyx/core/composable/PermanentChild.kt | 2 +- .../appyx/app/node/slideshow/slide/app/StatefulNode1.kt | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt index 28337c4e2..a572ef6ed 100644 --- a/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt +++ b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt @@ -13,7 +13,7 @@ fun ParentNode.PermanentChild( navTarget: NavTarget, decorator: @Composable (child: ChildRenderer) -> Unit ) { - val child = remember(navTarget) { + val child = remember(navTarget, permanentNavModel) { permanentNavModel .elements .value diff --git a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt index 913f3bbe6..ffce2a80b 100644 --- a/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt +++ b/samples/app/src/main/kotlin/com/bumble/appyx/app/node/slideshow/slide/app/StatefulNode1.kt @@ -42,6 +42,10 @@ import kotlinx.parcelize.Parcelize class StatefulNode1( buildContext: BuildContext, private val permanentNavModel: PermanentNavModel = PermanentNavModel( + NavTarget.Child(BASE_COUNTER), + NavTarget.Child(BASE_COUNTER * 2), + NavTarget.Child(BASE_COUNTER * 3), + NavTarget.Child(BASE_COUNTER * 4), savedStateMap = buildContext.savedStateMap ) ) : ParentNode( From 60eb43fde7201b76bd465bda1297fcd50049c89d Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Mon, 2 Oct 2023 11:43:52 +0100 Subject: [PATCH 4/4] Mention breaking change in the CHANGELOG.md and allow to add keys to PermanentNavModel later --- CHANGELOG.md | 2 +- .../appyx/core/node/PermanentChildTest.kt | 7 +++++ .../appyx/core/composable/PermanentChild.kt | 26 ++++++++++++------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a956fcef..aec6143e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Pending changes -- [#606](https://github.com/bumble-tech/appyx/pull/606) – **Fixed**: Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode +- [#606](https://github.com/bumble-tech/appyx/pull/606) – **Breaking change**: Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode --- diff --git a/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt b/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt index b9997e791..de13158bb 100644 --- a/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt +++ b/libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt @@ -38,6 +38,13 @@ class PermanentChildTest { rule.onNode(hasTestTag(TestParentNode.NavTarget::class.java.name)).assertExists() } + @Test + fun `WHEN_permanent_model_does_not_contain_relevant_nav_key_THEN_permanent_child_is_not_rendered`() { + rule.start() + + rule.onNode(hasTestTag(TestParentNode.NavTarget::class.java.name)).assertDoesNotExist() + } + @Test fun `WHEN_visibility_switched_THEN_permanent_child_is_reused`() { createPermanentNavModelWithNavKey() diff --git a/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt index a572ef6ed..0b809f23a 100644 --- a/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt +++ b/libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt @@ -1,11 +1,16 @@ package com.bumble.appyx.core.composable import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier +import com.bumble.appyx.core.mapState import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode +import kotlinx.coroutines.flow.SharingStarted @Composable fun ParentNode.PermanentChild( @@ -13,19 +18,22 @@ fun ParentNode.PermanentChild( navTarget: NavTarget, decorator: @Composable (child: ChildRenderer) -> Unit ) { - val child = remember(navTarget, permanentNavModel) { + val scope = rememberCoroutineScope() + val child by remember(navTarget, permanentNavModel) { permanentNavModel .elements - .value - .find { it.key.navTarget == navTarget } - ?.let { childOrCreate(it.key) } - ?: throw IllegalStateException( - "No child found for $navTarget in $permanentNavModel. " + - "Add $navTarget to $permanentNavModel before calling PermanentChild." - ) + // use WhileSubscribed or Lazy otherwise desynchronisation issue + .mapState(scope, SharingStarted.WhileSubscribed()) { navElements -> + navElements + .find { it.key.navTarget == navTarget } + ?.let { childOrCreate(it.key) } + } + }.collectAsState() + + child?.let { + decorator(PermanentChildRender(it.node)) } - decorator(PermanentChildRender(child.node)) } @Composable