diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index 22798482..6e30bbe3 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -97,7 +97,7 @@ jobs: name: Instrumentation tests needs: assemble runs-on: macos-latest - timeout-minutes: 20 + timeout-minutes: 30 strategy: # Allow tests to continue on other devices if they fail on one device. fail-fast: false diff --git a/core-compose/api/core-compose.api b/core-compose/api/core-compose.api index 2005694d..9e593426 100644 --- a/core-compose/api/core-compose.api +++ b/core-compose/api/core-compose.api @@ -28,6 +28,7 @@ public final class com/squareup/workflow/ui/compose/ComposeViewFactoryRoot$Compa } public final class com/squareup/workflow/ui/compose/ComposeViewFactoryRootKt { + public static final fun ()V public static final fun ComposeViewFactoryRoot (Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/compose/ComposeViewFactoryRoot; public static final fun withComposeViewFactoryRoot (Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewEnvironment; } diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryTest.kt new file mode 100644 index 00000000..510c4fbf --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright 2020 Square Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.workflow.ui.compose + +import android.content.Context +import android.widget.FrameLayout +import androidx.compose.FrameManager +import androidx.compose.mutableStateOf +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.ui.foundation.Text +import androidx.ui.layout.Column +import androidx.ui.test.assertIsDisplayed +import androidx.ui.test.createComposeRule +import androidx.ui.test.findByText +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.WorkflowViewStub +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ComposeViewFactoryTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun wrapsFactoryWithRoot() { + val wrapperText = mutableStateOf("one") + val viewEnvironment = ViewEnvironment(ViewRegistry(TestFactory)) + .withComposeViewFactoryRoot { content -> + Column { + Text(wrapperText.value) + content() + } + } + + composeRule.setContent { + // This is valid Compose code, but the IDE doesn't know that yet so it will show an + // unsuppressable error. + RootView(viewEnvironment = viewEnvironment) + } + + findByText("one\ntwo").assertIsDisplayed() + FrameManager.framed { + wrapperText.value = "ENO" + } + findByText("ENO\ntwo").assertIsDisplayed() + } + + private class RootView(context: Context) : FrameLayout(context) { + private val stub = WorkflowViewStub(context).also(::addView) + + fun setViewEnvironment(viewEnvironment: ViewEnvironment) { + stub.update(TestRendering("two"), viewEnvironment) + } + } + + private data class TestRendering(val text: String) + + private companion object { + val TestFactory = bindCompose { rendering, _ -> + Text(rendering.text) + } + } +} diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeSupportTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeSupportTest.kt new file mode 100644 index 00000000..0c6a10d1 --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeSupportTest.kt @@ -0,0 +1,96 @@ +/* + * Copyright 2020 Square Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +@file:Suppress("RemoveEmptyParenthesesFromAnnotationEntry") + +package com.squareup.workflow.ui.compose.internal + +import android.content.Context +import android.widget.FrameLayout +import androidx.compose.Composable +import androidx.compose.CompositionReference +import androidx.compose.FrameManager +import androidx.compose.Providers +import androidx.compose.Recomposer +import androidx.compose.ambientOf +import androidx.compose.compositionReference +import androidx.compose.currentComposer +import androidx.compose.mutableStateOf +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.ui.foundation.Text +import androidx.ui.test.assertIsDisplayed +import androidx.ui.test.createComposeRule +import androidx.ui.test.findByText +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ComposeSupportTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun ambientsPassThroughSubcomposition() { + composeRule.setContent { + TestComposable("foo") + } + + findByText("foo").assertIsDisplayed() + } + + @Test fun ambientChangesPassThroughSubcomposition() { + val ambientValue = mutableStateOf("foo") + composeRule.setContent { + TestComposable(ambientValue.value) + } + + findByText("foo").assertIsDisplayed() + FrameManager.framed { + ambientValue.value = "bar" + } + findByText("bar").assertIsDisplayed() + } + + @Composable private fun TestComposable(ambientValue: String) { + Providers(TestAmbient provides ambientValue) { + LegacyHostComposable { + Text(TestAmbient.current) + } + } + } + + @Composable private fun LegacyHostComposable(leafContent: @Composable() () -> Unit) { + val wormhole = Wormhole(currentComposer.recomposer, compositionReference(), leafContent) + // This is valid Compose code, but the IDE doesn't know that yet so it will show an + // unsuppressable error. + WormholeView(wormhole = wormhole) + } + + private class Wormhole( + val recomposer: Recomposer, + val parentReference: CompositionReference, + val childContent: @Composable() () -> Unit + ) + + private class WormholeView(context: Context) : FrameLayout(context) { + fun setWormhole(wormhole: Wormhole) { + setContent(wormhole.recomposer, wormhole.parentReference, wormhole.childContent) + } + } + + private companion object { + val TestAmbient = ambientOf { error("Ambient not provided") } + } +} diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt new file mode 100644 index 00000000..453c2545 --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt @@ -0,0 +1,142 @@ +/* + * Copyright 2020 Square Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.workflow.ui.compose.internal + +import androidx.compose.FrameManager +import androidx.compose.mutableStateOf +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.ui.foundation.Text +import androidx.ui.layout.Column +import androidx.ui.test.assertIsDisplayed +import androidx.ui.test.createComposeRule +import androidx.ui.test.findByText +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot +import com.squareup.workflow.ui.compose.wrapWithRootIfNecessary +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ComposeViewFactoryRootTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun wrapWithRootIfNecessary_handlesNoRoot() { + val viewEnvironment = ViewEnvironment(ViewRegistry()) + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment) { + Text("foo") + } + } + + findByText("foo").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_wrapsWhenNecessary() { + val viewEnvironment = ViewEnvironment(ViewRegistry()) + .withComposeViewFactoryRoot { content -> + Column { + Text("one") + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_onlyWrapsOnce() { + val viewEnvironment = ViewEnvironment(ViewRegistry()) + .withComposeViewFactoryRoot { content -> + Column { + Text("one") + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment) { + Text("two") + wrapWithRootIfNecessary(viewEnvironment) { + Text("three") + } + } + } + + findByText("one\ntwo\nthree").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_seesUpdatesFromRootWrapper() { + val wrapperText = mutableStateOf("one") + val viewEnvironment = ViewEnvironment(ViewRegistry()) + .withComposeViewFactoryRoot { content -> + Column { + Text(wrapperText.value) + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + FrameManager.framed { + wrapperText.value = "ENO" + } + findByText("ENO\ntwo").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_rewrapsWhenDifferentRoot() { + val viewEnvironment1 = ViewEnvironment(ViewRegistry()) + .withComposeViewFactoryRoot { content -> + Column { + Text("one") + content() + } + } + val viewEnvironment2 = ViewEnvironment(ViewRegistry()) + .withComposeViewFactoryRoot { content -> + Column { + Text("ENO") + content() + } + } + val viewEnvironment = mutableStateOf(viewEnvironment1) + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment.value) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + FrameManager.framed { + viewEnvironment.value = viewEnvironment2 + } + findByText("ENO\ntwo").assertIsDisplayed() + } +} diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRootTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt similarity index 87% rename from core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRootTest.kt rename to core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt index e27c6455..794caad8 100644 --- a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRootTest.kt +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.squareup.workflow.ui.compose +package com.squareup.workflow.ui.compose.internal import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.ui.foundation.Text @@ -23,16 +23,16 @@ import androidx.ui.test.assertIsDisplayed import androidx.ui.test.createComposeRule import androidx.ui.test.findByText import com.google.common.truth.Truth.assertThat -import com.squareup.workflow.ui.compose.internal.SafeComposeViewFactoryRoot +import com.squareup.workflow.ui.compose.ComposeViewFactoryRoot import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import kotlin.test.assertFailsWith @RunWith(AndroidJUnit4::class) -class ComposeViewFactoryRootTest { +class SafeComposeViewFactoryRootTest { - @Rule @JvmField val composeTestRule = createComposeRule() + @Rule @JvmField val composeRule = createComposeRule() @Test fun safeComposeViewFactoryRoot_wraps_content() { val wrapped = ComposeViewFactoryRoot { content -> @@ -43,7 +43,7 @@ class ComposeViewFactoryRootTest { } val safeRoot = SafeComposeViewFactoryRoot(wrapped) - composeTestRule.setContent { + composeRule.setContent { safeRoot.wrap { // Need an explicit semantics container, otherwise both Texts will be merged into a single // Semantics object with the text "Parent\nChild". @@ -53,8 +53,7 @@ class ComposeViewFactoryRootTest { } } - findByText("Parent") - .assertIsDisplayed() + findByText("Parent").assertIsDisplayed() findByText("Child").assertIsDisplayed() } @@ -63,7 +62,7 @@ class ComposeViewFactoryRootTest { val safeRoot = SafeComposeViewFactoryRoot(wrapped) val error = assertFailsWith { - composeTestRule.setContent { + composeRule.setContent { safeRoot.wrap {} } } @@ -82,7 +81,7 @@ class ComposeViewFactoryRootTest { val safeRoot = SafeComposeViewFactoryRoot(wrapped) val error = assertFailsWith { - composeTestRule.setContent { + composeRule.setContent { safeRoot.wrap { Text("Hello") } diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewFactoriesTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewFactoriesTest.kt new file mode 100644 index 00000000..1d4abc77 --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewFactoriesTest.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2020 Square Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.squareup.workflow.ui.compose.internal + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.ui.foundation.Text +import androidx.ui.layout.Column +import androidx.ui.test.assertIsDisplayed +import androidx.ui.test.createComposeRule +import androidx.ui.test.findByText +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ViewFactoriesTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun showRendering_wrapsFactoryWithRoot_whenAlreadyInComposition() { + val viewEnvironment = ViewEnvironment(ViewRegistry(TestFactory)) + .withComposeViewFactoryRoot { content -> + Column { + Text("one") + content() + } + } + + composeRule.setContent { + viewEnvironment.showRendering(TestRendering("two")) + } + + findByText("one\ntwo").assertIsDisplayed() + } + + private data class TestRendering(val text: String) + + private companion object { + val TestFactory = bindCompose { rendering, _ -> + Text(rendering.text) + } + } +} diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactory.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactory.kt index 2afb6797..9491b7e2 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactory.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactory.kt @@ -24,16 +24,12 @@ import android.view.ViewGroup import android.widget.FrameLayout import androidx.compose.Composable import androidx.compose.FrameManager -import androidx.compose.Recomposer import androidx.compose.StructurallyEqual import androidx.compose.mutableStateOf -import androidx.ui.core.setContent import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewFactory import com.squareup.workflow.ui.bindShowRendering -import com.squareup.workflow.ui.compose.internal.CompositionContinuation -import com.squareup.workflow.ui.compose.internal.SafeComposeViewFactoryRoot -import com.squareup.workflow.ui.compose.internal.setContent +import com.squareup.workflow.ui.compose.internal.setOrContinueContent import kotlin.reflect.KClass /** @@ -93,7 +89,7 @@ inline fun bindCompose( @PublishedApi internal class ComposeViewFactory( override val type: KClass, - internal val showRendering: @Composable() (RenderingT, ViewEnvironment) -> Unit + private val content: @Composable() (RenderingT, ViewEnvironment) -> Unit ) : ViewFactory { override fun buildView( @@ -133,41 +129,22 @@ internal class ComposeViewFactory( // Entry point to the world of Compose. composeContainer.setOrContinueContent(initialViewEnvironment) { val (rendering, environment) = renderState.value!! - showRendering(rendering, environment) + showRenderingWrappedWithRoot(rendering, environment) } return composeContainer } /** - * Starts composing [content] into this [ViewGroup]. - * - * It will either propagate the composition context from any outer [ComposeViewFactory]s, or if - * this is the first [ComposeViewFactory] in the tree, it will initialize it using the - * [ComposeViewFactoryRoot] if present. - * - * This function relies on [ViewFactory.showRendering] adding the [CompositionContinuation] to the - * [ViewEnvironment]. + * Invokes [content]. If this is the highest [ComposeViewFactory] in the tree, wraps with + * the [ComposeViewFactoryRoot] if present in the [ViewEnvironment]. */ - private fun ViewGroup.setOrContinueContent( - initialViewEnvironment: ViewEnvironment, - content: @Composable() () -> Unit + @Composable internal fun showRenderingWrappedWithRoot( + rendering: RenderingT, + viewEnvironment: ViewEnvironment ) { - val (compositionReference, recomposer) = initialViewEnvironment[CompositionContinuation] - if (compositionReference != null && recomposer != null) { - // Somewhere above us in the workflow rendering tree, there's another bindCompose factory. - // We need to link to its composition reference so we inherit its ambients. - setContent(recomposer, compositionReference, content) - } else { - // This is the first bindCompose factory in the rendering tree, so we need to initialize it - // with the ComposableDecorator if present. - val decorator = initialViewEnvironment[ComposeViewFactoryRoot] - val safeDecorator = SafeComposeViewFactoryRoot(decorator) - setContent(Recomposer.current()) { - safeDecorator.wrap { - content() - } - } + wrapWithRootIfNecessary(viewEnvironment) { + content(rendering, viewEnvironment) } } } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt index 25ed41e1..23163615 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt @@ -19,8 +19,17 @@ package com.squareup.workflow.ui.compose import androidx.compose.Composable import androidx.compose.Direct +import androidx.compose.Providers +import androidx.compose.remember +import androidx.compose.staticAmbientOf import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewEnvironmentKey +import com.squareup.workflow.ui.compose.internal.SafeComposeViewFactoryRoot + +/** + * Used by [wrapWithRootIfNecessary] to ensure the [ComposeViewFactoryRoot] is only applied once. + */ +private val HasViewFactoryRootBeenApplied = staticAmbientOf { false } /** * A `@Composable` function that is stored in a [ViewEnvironment] and will be used to wrap the first @@ -56,6 +65,34 @@ fun ComposeViewFactoryRoot( @Composable override fun wrap(content: @Composable() () -> Unit) = wrapper(content) } +/** + * Adds [content] to the composition, ensuring that any [ComposeViewFactoryRoot] present in the + * [ViewEnvironment] has been applied. Will only apply the root at the highest occurrence of this + * function in the composition subtree. + */ +@Composable internal fun wrapWithRootIfNecessary( + viewEnvironment: ViewEnvironment, + content: @Composable() () -> Unit +) { + if (HasViewFactoryRootBeenApplied.current) { + // The only way this ambient can have the value true is if, somewhere above this point in the + // composition, the else case below was hit and wrapped us in the ambient. Since the root + // wrapper will have already been applied, we can just compose content directly. + content() + } else { + // If the ambient is false, this is the first time this function has appeared in the composition + // so far. We provide a true value for the ambient for everything below us, so any recursive + // calls to this function will hit the if case above and not re-apply the wrapper. + Providers(HasViewFactoryRootBeenApplied provides true) { + val decorator = viewEnvironment[ComposeViewFactoryRoot] + val safeDecorator = remember(decorator) { + SafeComposeViewFactoryRoot(decorator) + } + safeDecorator.wrap(content) + } + } +} + private object NoopComposeViewFactoryRoot : ComposeViewFactoryRoot { @Direct @Composable override fun wrap(content: @Composable() () -> Unit) { content() diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ComposeSupport.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ComposeSupport.kt index 50621727..72c97918 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ComposeSupport.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ComposeSupport.kt @@ -64,8 +64,7 @@ internal fun ViewGroup.setContent( getChildAt(0).takeIf(ANDROID_OWNER_CLASS::isInstance) } else { removeAllViews(); null - } - ?: createOwner(context).also { addView(androidOwnerView(it), DefaultLayoutParams) } + } ?: createOwner(context).also { addView(androidOwnerView(it), DefaultLayoutParams) } return doSetContent(context, composeView, recomposer, parent, content) } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/CompositionContinuation.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt similarity index 52% rename from core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/CompositionContinuation.kt rename to core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt index 4d789b9a..0eaad52c 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/CompositionContinuation.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt @@ -13,13 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@file:Suppress("RemoveEmptyParenthesesFromAnnotationEntry") + package com.squareup.workflow.ui.compose.internal +import android.view.ViewGroup import androidx.compose.Composable import androidx.compose.CompositionReference import androidx.compose.Recomposer import androidx.compose.compositionReference import androidx.compose.currentComposer +import androidx.ui.core.setContent import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewEnvironmentKey @@ -33,26 +37,47 @@ import com.squareup.workflow.ui.ViewEnvironmentKey * [ComposeViewFactory] will then pull the continuation out of the environment and use it to link * its composition to the outer one. */ -internal data class CompositionContinuation( +internal data class ParentComposition( val reference: CompositionReference? = null, val recomposer: Recomposer? = null ) { - companion object : ViewEnvironmentKey( - CompositionContinuation::class - ) { - override val default: CompositionContinuation - get() = CompositionContinuation() + companion object : ViewEnvironmentKey(ParentComposition::class) { + override val default: ParentComposition get() = ParentComposition() } } /** - * Creates a [CompositionContinuation] from the current point in the composition and adds it to this + * Creates a [ParentComposition] from the current point in the composition and adds it to this * [ViewEnvironment]. */ -@Composable internal fun ViewEnvironment.withCompositionContinuation(): ViewEnvironment { - val compositionReference = CompositionContinuation( +@Composable internal fun ViewEnvironment.withParentComposition(): ViewEnvironment { + val compositionReference = ParentComposition( reference = compositionReference(), recomposer = currentComposer.recomposer ) - return this + (CompositionContinuation to compositionReference) + return this + (ParentComposition to compositionReference) +} + +/** + * Starts composing [content] into this [ViewGroup]. + * + * If there is a [ParentComposition] present in [initialViewEnvironment], it will start the + * composition as a subcomposition of that continuation. + * + * This function corresponds to [withParentComposition]. + */ +internal fun ViewGroup.setOrContinueContent( + initialViewEnvironment: ViewEnvironment, + content: @Composable() () -> Unit +) { + val (compositionReference, recomposer) = initialViewEnvironment[ParentComposition] + if (compositionReference != null && recomposer != null) { + // Somewhere above us in the workflow rendering tree, there's another bindCompose factory. + // We need to link to its composition reference so we inherit its ambients. + setContent(recomposer, compositionReference, content) + } else { + // This is the first bindCompose factory in the rendering tree, so it won't be a child + // composition. + setContent(Recomposer.current(), content) + } } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewFactories.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewFactories.kt index edd9290f..9007cd3e 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewFactories.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewFactories.kt @@ -36,7 +36,7 @@ import com.squareup.workflow.ui.compose.internal.ComposableViewStubWrapper.Updat * * *Note: [rendering] must be the same type as this [ViewFactory], even though the type system does * not enforce this constraint. This is due to a Compose compiler bug tracked - * [here](https://issuetracker.google.com/issues/156527332).* + * [here](https://issuetracker.google.com/issues/156527332). * * @see ViewEnvironment.showRendering * @see com.squareup.workflow.ui.ViewRegistry.showRendering @@ -47,19 +47,21 @@ import com.squareup.workflow.ui.compose.internal.ComposableViewStubWrapper.Updat viewEnvironment: ViewEnvironment, modifier: Modifier = Modifier ) { + val viewFactory = this Box(modifier = modifier) { // Fast path: If the child binding is also a Composable, we don't need to go through the legacy // view system and can just invoke the binding's composable function directly. - if (this is ComposeViewFactory) { - showRendering(rendering, viewEnvironment) + if (viewFactory is ComposeViewFactory) { + viewFactory.showRenderingWrappedWithRoot(rendering, viewEnvironment) } else { - // Plumb the current composition "context" through the ViewEnvironment so any nested composable - // factories get access to any ambients currently in effect. + // Plumb the current composition "context" through the ViewEnvironment so any nested + // composable factories get access to any ambients currently in effect. // See setOrContinueContent(). - val newEnvironment = viewEnvironment.withCompositionContinuation() + val newEnvironment = viewEnvironment.withParentComposition() - // IntelliJ currently complains very loudly about this function call, but it actually compiles. - // The IDE tooling isn't currently able to recognize that the Compose compiler accepts this code. + // IntelliJ currently complains very loudly about this function call, but it actually + // compiles. The IDE tooling isn't currently able to recognize that the Compose compiler + // accepts this code. ComposableViewStubWrapper(update = Update(rendering, newEnvironment)) } }