From 56165a1c463df7ab5e009f08957f89fe082b062d Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Fri, 15 May 2020 18:27:56 -0700 Subject: [PATCH 01/15] Upgrade gradle to latest version. --- gradle/wrapper/gradle-wrapper.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index fd0c5a38..21e622da 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.4-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-6.4.1-all.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists From acfbee02360106aaefcf522582370792b5d3d886 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Fri, 15 May 2020 13:24:04 -0700 Subject: [PATCH 02/15] Make ViewFactory.showRendering function responsible for applying the ComposeViewFactoryRoot. This ensures that the root will be applied in all code paths: - Entering the Compose world through a `WorkflowViewStub`/`ComposeViewFactory`. - Already in Compose, showing a rendering from a `WorkflowContainer`. This change also ensures that if the `ComposeViewFactoryRoot` is changed above where it's applied, the new wrapper will be applied. Adds tests for this and other logic. Fixes #20. --- .github/workflows/kotlin.yml | 2 +- core-compose/api/core-compose.api | 1 + .../ui/compose/ComposeViewFactoryTest.kt | 78 ++++++++++ .../ui/compose/internal/ComposeSupportTest.kt | 96 ++++++++++++ .../internal/ComposeViewFactoryRootTest.kt | 142 ++++++++++++++++++ .../SafeComposeViewFactoryRootTest.kt} | 17 +-- .../ui/compose/internal/ViewFactoriesTest.kt | 61 ++++++++ .../workflow/ui/compose/ComposeViewFactory.kt | 43 ++---- .../ui/compose/ComposeViewFactoryRoot.kt | 37 +++++ .../ui/compose/internal/ComposeSupport.kt | 3 +- ...onContinuation.kt => ParentComposition.kt} | 45 ++++-- .../ui/compose/internal/ViewFactories.kt | 18 ++- 12 files changed, 480 insertions(+), 63 deletions(-) create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ComposeViewFactoryTest.kt create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeSupportTest.kt create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt rename core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/{ComposeViewFactoryRootTest.kt => internal/SafeComposeViewFactoryRootTest.kt} (87%) create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewFactoriesTest.kt rename core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/{CompositionContinuation.kt => ParentComposition.kt} (52%) 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)) } } From 9a6c51ad48130edc7393cba863b17a0d47bd2284 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 13:46:02 -0700 Subject: [PATCH 03/15] Fix gradle cache in CI. Most of the tests in this repo are UI tests, so it would be really nice to not build the entire codebase twice to start running them. I decreased the timeout as well since test shards no longer need to re-build. --- .github/workflows/kotlin.yml | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index 6e30bbe3..ece141dc 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -6,10 +6,7 @@ on: pull_request: env: - # Must use $HOME here, NOT a tilde, because of the order in which bash does expansion: - # Tilde happens before variables, so will be used literally, whereas $HOME will be - # recursively expanded. - GRADLE_CACHE_PATH: $HOME/.gradle/caches + GRADLE_HOME: ${{ github.workspace }}/gradle-home jobs: assemble: @@ -29,7 +26,7 @@ jobs: - name: Cache gradle dependencies uses: actions/cache@v1 with: - path: ${{ env.GRADLE_CACHE_PATH }} + path: ${{ env.GRADLE_HOME }}/caches # Include the SHA in the hash so this step always adds a cache entry. If we didn't use the SHA, the artifacts # would only get cached once for each build config hash. # Don't use ${{ runner.os }} in the key so we don't re-assemble for UI tests. @@ -47,13 +44,13 @@ jobs: # and there's no way to modify the cache after the job that created it finishes. - name: Clean gradle build cache to assemble fresh run: | - ls -lhrt $GRADLE_CACHE_PATH || true - rm -rf $GRADLE_CACHE_PATH/build-cache-1 - ls -lhrt $GRADLE_CACHE_PATH || true + ls -lhrt "$GRADLE_HOME/caches" || true + rm -rf "$GRADLE_HOME/caches/build-cache-1" + ls -lhrt "$GRADLE_HOME/caches" || true ## Actual task - name: Assemble with gradle - run: ./gradlew assemble --build-cache --no-daemon --stacktrace + run: ./gradlew assemble --build-cache --no-daemon --stacktrace --gradle-user-home "$GRADLE_HOME" # Runs all check tasks in parallel. check: @@ -84,14 +81,14 @@ jobs: - name: Cache build artifacts uses: actions/cache@v1 with: - path: ${{ env.GRADLE_CACHE_PATH }} + path: ${{ env.GRADLE_HOME }}/caches # Don't set restore-keys so cache is always only valid for the current build config. # Also don't use ${{ runner.os }} in the key so we don't re-assemble for UI tests. key: gradle-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/buildSrc/**') }}-${{ github.sha }} ## Actual task - name: Check with Gradle - run: ./gradlew ${{ matrix.gradle-task }} --build-cache --no-daemon --stacktrace + run: ./gradlew ${{ matrix.gradle-task }} --build-cache --no-daemon --stacktrace --gradle-user-home "$GRADLE_HOME" instrumentation-tests: name: Instrumentation tests @@ -119,7 +116,7 @@ jobs: - name: Cache build artifacts uses: actions/cache@v1 with: - path: ${{ env.GRADLE_CACHE_PATH }} + path: ${{ env.GRADLE_HOME }}/caches # Don't set restore-keys so cache is always only valid for the current build config. # Also don't use ${{ runner.os }} in the key so we don't re-assemble for UI tests. key: gradle-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/*.gradle*') }}-${{ hashFiles('**/buildSrc/**') }}-${{ github.sha }} @@ -130,4 +127,4 @@ jobs: with: api-level: ${{ matrix.api-level }} arch: x86_64 - script: ./gradlew connectedCheck --build-cache --no-daemon --stacktrace + script: ./gradlew connectedCheck --build-cache --no-daemon --stacktrace --gradle-user-home "$GRADLE_HOME" From f91253ab5a04c62ec846e174a35f3decee84b6b2 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 13:59:54 -0700 Subject: [PATCH 04/15] Upload UI test reports as Github Actions artifacts. --- .github/workflows/kotlin.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index ece141dc..249f19f8 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -128,3 +128,8 @@ jobs: api-level: ${{ matrix.api-level }} arch: x86_64 script: ./gradlew connectedCheck --build-cache --no-daemon --stacktrace --gradle-user-home "$GRADLE_HOME" + - name: Upload results + uses: actions/upload-artifact@v2 + with: + name: instrumentation-test-results + path: ./**/build/reports/androidTests/connected/** From eb8a3d0078201ba3f93670a89de8c758225b6425 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 14:16:15 -0700 Subject: [PATCH 05/15] Releasing v0.29.0. --- CHANGELOG.md | 15 +++++++ RELEASING.md | 104 ++++++++++++++++++++++++++++++++++++++++++++++ gradle.properties | 2 +- 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.md create mode 100644 RELEASING.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..1c1747eb --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,15 @@ +Change Log +========== + +Version 0.29.0 +-------------- + +_2020-05-18_ + +* First release from separate repo. +* Update: Compose version to dev11. (#26) +* New: Add the ability to display nested renderings with `bindCompose`. (#7) +* New: Introduce `ComposeWorkflow`, a self-rendering Workflow. (#8) +* New: Introduce tooling module with support for previewing ViewBindings with Compose's Preview. (#15) +* New: Introduce WorkflowContainer for running a workflow inside a Compose app. (#16) +* Breaking: Tidy up the package structure. (#23) diff --git a/RELEASING.md b/RELEASING.md new file mode 100644 index 00000000..6c0ff1b0 --- /dev/null +++ b/RELEASING.md @@ -0,0 +1,104 @@ +Releasing +========= + +Production Releases +------------------- + +1. Merge an update of [the change log](CHANGELOG.md) with the changes since the last release. + +1. Make sure you're on the `master` branch (or fix branch, e.g. `v0.1-fixes`). + +1. Confirm that the kotlin build is green before committing any changes + ```bash + (cd kotlin && ./gradlew build connectedCheck) + ``` + +1. In `kotlin/gradle.properties`, remove the `-SNAPSHOT` prefix from the `VERSION_NAME` property. + E.g. `VERSION_NAME=0.1.0` + +1. Create a commit and tag the commit with the version number: + ```bash + git commit -am "Releasing v0.1.0." + git tag v0.1.0 + ``` + +1. Upload the kotlin artifacts: + ```bash + (cd kotlin && ./gradlew build && ./gradlew uploadArchives --no-parallel) + ``` + +1. Close and release the staging repository at https://oss.sonatype.org. + +1. Update the `VERSION_NAME` property in `kotlin/gradle.properties` to the new + snapshot version, e.g. `VERSION_NAME=0.2.0-SNAPSHOT`. + +1. Commit the new snapshot version: + ``` + git commit -am "Finish releasing v0.1.0." + ``` + +1. Push your commits and tag: + ``` + git push origin master + # or git push origin fix-branch + git push origin v0.1.0 + ``` + +1. Create the release on GitHub: + 1. Go to the [Releases](https://github.com/square/workflow-kotlin-compose/releases) page for the GitHub + project. + 1. Click "Draft a new release". + 1. Enter the tag name you just pushed. + 1. Title the release with the same name as the tag. + 1. Copy & paste the changelog entry for this release into the description. + 1. If this is a pre-release version, check the pre-release box. + 1. Hit "Publish release". + +1. If this was a fix release, merge changes to the master branch: + ```bash + git checkout master + git pull + git merge --no-ff v0.1-fixes + # Resolve conflicts. Accept master's versions of gradle.properties and podspecs. + git push origin master + ``` + +1. Publish the website. See below. + +--- + +## Kotlin Notes + +### Development + +To build and install the current version to your local Maven repository (`~/.m2`), run: + +```bash +./gradlew clean installArchives +``` + +### Deploying + +#### Configuration + +In order to deploy artifacts to a Maven repository, you'll need to set 4 properties in your private +Gradle properties file (`~/.gradle/gradle.properties`): + +``` +RELEASE_REPOSITORY_URL= +SNAPSHOT_REPOSITORY_URL= +SONATYPE_NEXUS_PASSWORD= +``` + +#### Snapshot Releases + +Double-check that `gradle.properties` correctly contains the `-SNAPSHOT` suffix, then upload +snapshot artifacts to Sonatype just like you would for a production release: + +```bash +./gradlew clean build && ./gradlew uploadArchives --no-parallel +``` + +You can verify the artifacts are available by visiting +https://oss.sonatype.org/content/repositories/snapshots/com/squareup/workflow/. diff --git a/gradle.properties b/gradle.properties index f3dd0091..4f9a71c9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -23,7 +23,7 @@ android.enableJetifier=true systemProp.org.gradle.internal.publish.checksums.insecure=true GROUP=com.squareup.workflow -VERSION_NAME=0.27.0-SNAPSHOT +VERSION_NAME=0.29.0 POM_DESCRIPTION=Reactive workflows From 41cbbf1fe8b101fab33be1a7d3461dcfa511e331 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 15:48:07 -0700 Subject: [PATCH 06/15] Finish releasing v0.29.0. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 4f9a71c9..3778eecb 100644 --- a/gradle.properties +++ b/gradle.properties @@ -23,7 +23,7 @@ android.enableJetifier=true systemProp.org.gradle.internal.publish.checksums.insecure=true GROUP=com.squareup.workflow -VERSION_NAME=0.29.0 +VERSION_NAME=0.30.0-SNAPSHOT POM_DESCRIPTION=Reactive workflows From 4c1c33b073eaedaa5a0e5625701f3e829353a8ce Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 16:09:08 -0700 Subject: [PATCH 07/15] Update README with dependency instructions for v0.29.0. --- README.md | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3d278fdf..50c5ffc5 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # workflow-kotlin-compose +[![GitHub license](https://img.shields.io/badge/license-Apache%20License%202.0-blue.svg?style=flat)](https://www.apache.org/licenses/LICENSE-2.0) +[![Maven Central](https://img.shields.io/maven-central/v/com.squareup.workflow/workflow-ui-core-compose.svg?label=Maven%20Central)](https://search.maven.org/search?q=g:com.squareup.workflow%20AND%20a:workflow-ui-core-compose) + This module provides experimental support for [Jetpack Compose UI][1] with workflows. The only integration that is currently supported is the ability to define [ViewFactories][2] that @@ -20,8 +23,24 @@ and to experiment with various ways to integrate Compose with Workflow. ## Usage -To get started, you must be using the latest Android Gradle Plugin 4.x version. Then, you need to -enable Compose support in your `build.gradle`: +### Add the dependency + +Add the dependencies from this project (they're on Maven Central): + +```groovy +dependencies { + // Main dependency + implementation "com.squareup.workflow:workflow-ui-core-compose:${versions.workflow_compose}" + + // For the preview helpers + implementation "com.squareup.workflow:workflow-ui-compose-tooling:${versions.workflow_compose}" +} +``` + +### Enable Compose + +You must be using the latest Android Gradle Plugin 4.x version, and enable Compose support +in your `build.gradle`: ```groovy android { @@ -47,7 +66,7 @@ To create a `ViewFactory`, call `bindCompose`. The lambda passed to `bindCompose function. ```kotlin -val HelloBinding = bindCompose { rendering -> +val HelloBinding = bindCompose { rendering, _ -> MaterialTheme { Clickable(onClick = { rendering.onClick() }) { Text(rendering.message) From da4527c85b33f45207de2f85c881d4a03be09309 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Sat, 16 May 2020 19:30:11 -0700 Subject: [PATCH 08/15] Inline WorkflowViewStub logic in ViewFactory.showRendering to avoid unnecessary intermediate Views. This is effectively the logic of `WorkflowViewStub`, but translated into Compose idioms. This approach has a few advantages: - Avoids extra custom views required to host `WorkflowViewStub` inside a Composition. Its trick of replacing itself in its parent doesn't play nice with Compose. - Allows us to pass the correct parent view for inflation (the root of the composition). - Avoids `WorkflowViewStub` having to do its own lookup to find the correct [ViewFactory], since we already have the correct one. Like `WorkflowViewStub`, this function uses the `ViewFactory` to create and memoize a `View` to display the rendering, keeps it updated with the latest `RenderingT` and `ViewEnvironment`, and adds it to the composition. --- .buildscript/android-ui-tests.gradle | 3 + .github/workflows/kotlin.yml | 4 +- .../workflow/ui/compose/ComposeViewFactory.kt | 21 ++-- .../ui/compose/internal/ParentComposition.kt | 40 ++++---- .../ui/compose/internal/ViewFactories.kt | 97 ++++++++++++------- .../nestedrenderings/NestedRenderingsTest.kt | 26 ++++- 6 files changed, 121 insertions(+), 70 deletions(-) diff --git a/.buildscript/android-ui-tests.gradle b/.buildscript/android-ui-tests.gradle index 120e3c2e..ffa0eae3 100644 --- a/.buildscript/android-ui-tests.gradle +++ b/.buildscript/android-ui-tests.gradle @@ -2,6 +2,9 @@ android { defaultConfig { testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } + testOptions { + animationsDisabled = true + } } dependencies { diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index 249f19f8..73804b48 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -100,9 +100,7 @@ jobs: fail-fast: false matrix: api-level: - # Tests are failing on APIs <24. - #- 21 - #- 23 + - 21 - 24 - 29 steps: 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 9491b7e2..6bbb6c7b 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 @@ -29,7 +29,8 @@ import androidx.compose.mutableStateOf 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.setOrContinueContent +import com.squareup.workflow.ui.compose.internal.ParentComposition +import com.squareup.workflow.ui.compose.internal.setOrSubcomposeContent import kotlin.reflect.KClass /** @@ -111,23 +112,25 @@ internal class ComposeViewFactory( areEquivalent = StructurallyEqual ) - // Models will throw if their properties are accessed when there is no frame open. Currently, - // that will be the case if the model is accessed before any other Compose infrastructure has - // ran, i.e. if this view factory is the first compose code to run in the app. - // I believe that eventually there will be a global frame that will make this unnecessary. - FrameManager.ensureStarted() - // Update the state whenever a new rendering is emitted. composeContainer.bindShowRendering( initialRendering, initialViewEnvironment ) { rendering, environment -> // This lambda will be executed synchronously before bindShowRendering returns. - renderState.value = Pair(rendering, environment) + + // Models will throw if their properties are accessed when there is no frame open. Currently, + // that will be the case if the model is accessed before any other Compose infrastructure has + // run, i.e. if this view factory is the first compose code to run in the app. + // I believe that eventually there will be a global frame that will make this unnecessary. + FrameManager.framed { + renderState.value = Pair(rendering, environment) + } } // Entry point to the world of Compose. - composeContainer.setOrContinueContent(initialViewEnvironment) { + val parentComposition = initialViewEnvironment[ParentComposition] + composeContainer.setOrSubcomposeContent(parentComposition.reference) { val (rendering, environment) = renderState.value!! showRenderingWrappedWithRoot(rendering, environment) } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt index 0eaad52c..633530e8 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt @@ -22,24 +22,22 @@ 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 /** - * Holds a [CompositionReference] and a [Recomposer] that can be used to [setContent] to create a - * composition that is a child of another composition. Child compositions get ambients and other - * compose context from their parent, which allows ambients provided around a [showRendering] call - * to be read by nested [bindCompose] factories. + * Holds a [CompositionReference] and that can be passed to [setOrSubcomposeContent] to create a + * composition that is a child of another composition. Subcompositions get ambients and other + * compose context from their parent, and propagate invalidations, which allows ambients provided + * around a [showRendering] call to be read by nested Compose-based view factories. * * When [showRendering] is called, it will store an instance of this class in the [ViewEnvironment]. - * [ComposeViewFactory] will then pull the continuation out of the environment and use it to link - * its composition to the outer one. + * [ComposeViewFactory] pulls the reference out of the environment and uses it to link its + * composition to the outer one. */ -internal data class ParentComposition( - val reference: CompositionReference? = null, - val recomposer: Recomposer? = null +internal class ParentComposition( + var reference: CompositionReference? = null ) { companion object : ViewEnvironmentKey(ParentComposition::class) { override val default: ParentComposition get() = ParentComposition() @@ -50,31 +48,29 @@ internal data class ParentComposition( * Creates a [ParentComposition] from the current point in the composition and adds it to this * [ViewEnvironment]. */ -@Composable internal fun ViewEnvironment.withParentComposition(): ViewEnvironment { - val compositionReference = ParentComposition( - reference = compositionReference(), - recomposer = currentComposer.recomposer - ) +@Composable internal fun ViewEnvironment.withParentComposition( + reference: CompositionReference = compositionReference() +): ViewEnvironment { + val compositionReference = ParentComposition(reference = reference) 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. + * If [parentComposition] is not null, [content] will be installed as a _subcomposition_ of the + * parent composition, meaning that it will propagate ambients and invalidation. * * This function corresponds to [withParentComposition]. */ -internal fun ViewGroup.setOrContinueContent( - initialViewEnvironment: ViewEnvironment, +internal fun ViewGroup.setOrSubcomposeContent( + parentComposition: CompositionReference?, content: @Composable() () -> Unit ) { - val (compositionReference, recomposer) = initialViewEnvironment[ParentComposition] - if (compositionReference != null && recomposer != null) { + if (parentComposition != 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) + setContent(Recomposer.current(), parentComposition, content) } else { // This is the first bindCompose factory in the rendering tree, so it won't be a child // composition. 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 9007cd3e..70048c41 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 @@ -15,17 +15,24 @@ */ package com.squareup.workflow.ui.compose.internal -import android.content.Context -import android.view.ViewGroup.LayoutParams.MATCH_PARENT -import android.widget.FrameLayout +import android.view.View +import android.view.ViewGroup import androidx.compose.Composable +import androidx.compose.compositionReference +import androidx.compose.onPreCommit +import androidx.compose.remember +import androidx.ui.core.AndroidOwner +import androidx.ui.core.ContextAmbient import androidx.ui.core.Modifier +import androidx.ui.core.OwnerAmbient +import androidx.ui.core.Ref import androidx.ui.foundation.Box +import androidx.ui.viewinterop.AndroidView import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewFactory -import com.squareup.workflow.ui.WorkflowViewStub +import com.squareup.workflow.ui.canShowRendering import com.squareup.workflow.ui.compose.ComposeViewFactory -import com.squareup.workflow.ui.compose.internal.ComposableViewStubWrapper.Update +import com.squareup.workflow.ui.showRendering /** * Renders [rendering] into the composition using the `ViewRegistry` from the [ViewEnvironment] to @@ -49,49 +56,73 @@ import com.squareup.workflow.ui.compose.internal.ComposableViewStubWrapper.Updat ) { 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. + // "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 (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. - // See setOrContinueContent(). - 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. - ComposableViewStubWrapper(update = Update(rendering, newEnvironment)) + return@Box } + + // "Slow" path: Create a legacy Android View to show the rendering, like WorkflowViewStub. + ViewFactoryAndroidView(viewFactory, rendering, viewEnvironment) } } /** - * Wraps a [WorkflowViewStub] with an API that is more Compose-friendly. + * This is effectively the logic of [com.squareup.workflow.ui.WorkflowViewStub], but translated + * into Compose idioms. This approach has a few advantages: + * + * - Avoids extra custom views required to host `WorkflowViewStub` inside a Composition. Its trick + * of replacing itself in its parent doesn't play nice with Compose. + * - Allows us to pass the correct parent view for inflation (the root of the composition). + * - Avoids `WorkflowViewStub` having to do its own lookup to find the correct [ViewFactory], since + * we already have the correct one. * - * In particular, Compose will only generate `Emittable`s for views with a single constructor - * that takes a [Context]. + * Like `WorkflowViewStub`, this function uses the [viewFactory] to create and memoize a [View] to + * display the [rendering], keeps it updated with the latest [rendering] and [viewEnvironment], and + * adds it to the composition. * - * See [this slack message](https://kotlinlang.slack.com/archives/CJLTWPH7S/p1576264533012000?thread_ts=1576262311.008800&cid=CJLTWPH7S). + * This function also passes a [ParentComposition] down through the [ViewEnvironment] so that if the + * child view further nests any `ComposableViewFactory`s, they will be correctly subcomposed. */ -private class ComposableViewStubWrapper(context: Context) : FrameLayout(context) { +@Composable private fun ViewFactoryAndroidView( + viewFactory: ViewFactory, + rendering: R, + viewEnvironment: ViewEnvironment +) { + val childView = remember { Ref() } - data class Update( - val rendering: Any, - val viewEnvironment: ViewEnvironment - ) + // Plumb the current composition through the ViewEnvironment so any nested composable factories + // get access to any ambients currently in effect. See setOrSubcomposeContent(). + val parentComposition = remember { ParentComposition() } + parentComposition.reference = compositionReference() + val wrappedEnvironment = remember(viewEnvironment) { + viewEnvironment + (ParentComposition to parentComposition) + } - private val viewStub = WorkflowViewStub(context) + // A view factory can decide to recreate its view at any time. This also covers the case where + // the value of the viewFactory argument has changed, including to one with a different type. + if (childView.value?.canShowRendering(rendering) != true) { + // If we don't pass the parent Android View, the child will have the wrong LayoutParams. + // OwnerAmbient is deprecated, but the only way to get the root view currently. I've filed + // a feature request to expose this as first-class API, see + // https://issuetracker.google.com/issues/156875705. + @Suppress("DEPRECATION") + val parentView = (OwnerAmbient.current as? AndroidOwner)?.view as? ViewGroup - init { - addView(viewStub) + childView.value = viewFactory.buildView( + initialRendering = rendering, + initialViewEnvironment = wrappedEnvironment, + contextForNewView = ContextAmbient.current, + container = parentView + ) } - // Compose turns this into a parameter when you invoke this class as a Composable. - fun setUpdate(update: Update) { - viewStub.update(update.rendering, update.viewEnvironment) + // Invoke the ViewFactory's update logic whenever the view, the rendering, or the ViewEnvironment + // change. + onPreCommit(childView.value, rendering, wrappedEnvironment) { + childView.value!!.showRendering(rendering, wrappedEnvironment) } - override fun getLayoutParams(): LayoutParams = LayoutParams(MATCH_PARENT, MATCH_PARENT) + AndroidView(childView.value!!) } diff --git a/samples/nested-renderings/src/androidTest/java/com/squareup/sample/nestedrenderings/NestedRenderingsTest.kt b/samples/nested-renderings/src/androidTest/java/com/squareup/sample/nestedrenderings/NestedRenderingsTest.kt index c2014b74..a4440c47 100644 --- a/samples/nested-renderings/src/androidTest/java/com/squareup/sample/nestedrenderings/NestedRenderingsTest.kt +++ b/samples/nested-renderings/src/androidTest/java/com/squareup/sample/nestedrenderings/NestedRenderingsTest.kt @@ -24,7 +24,6 @@ import androidx.ui.test.assertIsDisplayed import androidx.ui.test.doClick import androidx.ui.test.findAllByText import androidx.ui.test.findByText -import androidx.ui.test.last import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -49,11 +48,32 @@ class NestedRenderingsTest { findAllByText(ADD_BUTTON_TEXT) .assertCountEquals(4) - findAllByText("Reset").last() - .doClick() + resetAll() findAllByText(ADD_BUTTON_TEXT).assertCountEquals(1) } + /** + * We can't rely on the order of nodes returned by [findAllByText], and the contents of the + * collection will change as we remove nodes, so we have to double-loop over all reset buttons and + * click them all until there is only one left. + */ + private fun resetAll() { + var foundNodes = Int.MAX_VALUE + while (foundNodes > 1) { + foundNodes = 0 + findAllByText("Reset").forEach { + try { + it.assertExists() + } catch (e: AssertionError) { + // No more reset buttons, we're done. + return@forEach + } + foundNodes++ + it.doClick() + } + } + } + private fun SemanticsNodeInteractionCollection.forEach( block: (SemanticsNodeInteraction) -> Unit ) { From 5e620d7a7af8c9a49852575d5d00c55c0af2ed1d Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Tue, 19 May 2020 15:37:12 -0700 Subject: [PATCH 09/15] Make ViewRegistry.showRendering update if ViewRegistry changes. --- .../ui/compose/internal/ViewRegistriesTest.kt | 56 +++++++++++++++++++ .../ui/compose/internal/ViewRegistries.kt | 2 +- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt new file mode 100644 index 00000000..f6302b12 --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt @@ -0,0 +1,56 @@ +/* + * 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.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 org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ViewRegistriesTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun showRendering_recomposes_whenFactoryChanged() { + val registry1 = ViewRegistry(bindCompose { rendering, _ -> + Text(rendering) + }) + val registry2 = ViewRegistry(bindCompose { rendering, _ -> + Text(rendering.reversed()) + }) + val registry = mutableStateOf(registry1) + + composeRule.setContent { + registry.value.showRendering("hello", ViewEnvironment(registry.value)) + } + + findByText("hello").assertIsDisplayed() + FrameManager.framed { + registry.value = registry2 + } + findByText("olleh").assertIsDisplayed() + } +} diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt index 80f6f2aa..cc456c35 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt @@ -37,6 +37,6 @@ import com.squareup.workflow.ui.ViewRegistry modifier: Modifier = Modifier ) { val renderingType = rendering::class - val viewFactory = remember(renderingType) { getFactoryFor(renderingType) } + val viewFactory = remember(this, renderingType) { getFactoryFor(renderingType) } viewFactory.showRendering(rendering, hints, modifier) } From 3bb2f1f4e32b58e3c74d7d5876f59c4699f260bc Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Tue, 19 May 2020 16:56:47 -0700 Subject: [PATCH 10/15] Rename bindCompose to composedViewFactory. Fixes #19. --- README.md | 8 ++++---- .../ui/compose/tooling/PreviewViewFactoryTest.kt | 10 +++++----- .../ui/compose/tooling/PlaceholderViewFactory.kt | 4 ++-- .../workflow/ui/compose/ComposeViewFactoryTest.kt | 2 +- .../ui/compose/internal/ViewFactoriesTest.kt | 4 ++-- .../squareup/workflow/ui/compose/ComposeRendering.kt | 2 +- .../workflow/ui/compose/ComposeViewFactory.kt | 12 ++++++------ .../workflow/ui/compose/ComposeViewFactoryRoot.kt | 12 ++++++------ .../squareup/workflow/ui/compose/ComposeWorkflow.kt | 2 +- .../squareup/workflow/ui/compose/ViewEnvironments.kt | 2 +- .../ui/compose/internal/ParentComposition.kt | 4 ++-- .../workflow/ui/compose/internal/ViewFactories.kt | 3 ++- .../workflow/ui/compose/internal/ViewRegistries.kt | 2 +- .../sample/hellocomposebinding/HelloBinding.kt | 4 ++-- .../com/squareup/sample/hellocompose/HelloBinding.kt | 4 ++-- .../sample/nestedrenderings/RecursiveViewFactory.kt | 4 ++-- 16 files changed, 40 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 50c5ffc5..521d5b13 100644 --- a/README.md +++ b/README.md @@ -62,11 +62,11 @@ tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach { } ``` -To create a `ViewFactory`, call `bindCompose`. The lambda passed to `bindCompose` is a `@Composable` -function. +To create a `ViewFactory`, call `composedViewFactory`. The lambda passed to `composedViewFactory` is +a `@Composable` function. ```kotlin -val HelloBinding = bindCompose { rendering, _ -> +val HelloBinding = composedViewFactory { rendering, _ -> MaterialTheme { Clickable(onClick = { rendering.onClick() }) { Text(rendering.message) @@ -75,7 +75,7 @@ val HelloBinding = bindCompose { rendering, _ -> } ``` -The `bindCompose` function returns a regular [`ViewFactory`][2] which can be added to a +The `composedViewFactory` function returns a regular [`ViewFactory`][2] which can be added to a [`ViewRegistry`][3] like any other: ```kotlin diff --git a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt index cab900a5..b09ff3bd 100644 --- a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt +++ b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt @@ -31,7 +31,7 @@ import androidx.ui.test.findByText import androidx.ui.tooling.preview.Preview import androidx.ui.unit.dp import com.squareup.workflow.ui.ViewEnvironmentKey -import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.composedViewFactory import com.squareup.workflow.ui.compose.showRendering import org.junit.Rule import org.junit.Test @@ -99,7 +99,7 @@ class PreviewViewFactoryTest { findByText("foo").assertIsDisplayed() } - private val ParentWithOneChild = bindCompose> { rendering, environment -> + private val ParentWithOneChild = composedViewFactory> { rendering, environment -> Column { Text(rendering.first) Semantics(container = true, mergeAllDescendants = true) { @@ -113,7 +113,7 @@ class PreviewViewFactoryTest { } private val ParentWithTwoChildren = - bindCompose> { rendering, environment -> + composedViewFactory> { rendering, environment -> Column { Semantics(container = true) { environment.showRendering(rendering = rendering.first) @@ -134,7 +134,7 @@ class PreviewViewFactoryTest { val child: RecursiveRendering? = null ) - private val ParentRecursive = bindCompose { rendering, environment -> + private val ParentRecursive = composedViewFactory { rendering, environment -> Column { Text(rendering.text) rendering.child?.let { child -> @@ -175,7 +175,7 @@ class PreviewViewFactoryTest { override val default: String get() = error("Not specified") } - private val ParentConsumesCustomKey = bindCompose { _, environment -> + private val ParentConsumesCustomKey = composedViewFactory { _, environment -> Text(environment[TestEnvironmentKey]) } diff --git a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt index fae90a78..9987b25b 100644 --- a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt +++ b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt @@ -42,14 +42,14 @@ import androidx.ui.unit.dp import androidx.ui.unit.px import androidx.ui.unit.toRect import com.squareup.workflow.ui.ViewFactory -import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.composedViewFactory /** * A [ViewFactory] that will be used any time a [PreviewViewRegistry] is asked to show a rendering. * It displays a placeholder graphic and the rendering's `toString()` result. */ internal fun placeholderViewFactory(modifier: Modifier): ViewFactory = - bindCompose { rendering, _ -> + composedViewFactory { rendering, _ -> Text( modifier = modifier .clipToBounds() 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 index 510c4fbf..af6a87c8 100644 --- 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 @@ -71,7 +71,7 @@ class ComposeViewFactoryTest { private data class TestRendering(val text: String) private companion object { - val TestFactory = bindCompose { rendering, _ -> + val TestFactory = composedViewFactory { rendering, _ -> Text(rendering.text) } } 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 index 1d4abc77..110a76b5 100644 --- 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 @@ -23,7 +23,7 @@ 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.composedViewFactory import com.squareup.workflow.ui.compose.showRendering import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot import org.junit.Rule @@ -54,7 +54,7 @@ class ViewFactoriesTest { private data class TestRendering(val text: String) private companion object { - val TestFactory = bindCompose { rendering, _ -> + val TestFactory = composedViewFactory { rendering, _ -> Text(rendering.text) } } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeRendering.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeRendering.kt index 466fcafd..7e1b97ce 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeRendering.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeRendering.kt @@ -38,7 +38,7 @@ class ComposeRendering internal constructor( /** * A [ViewFactory] that renders a [ComposeRendering]. */ - val Factory: ViewFactory = bindCompose { rendering, environment -> + val Factory: ViewFactory = composedViewFactory { rendering, environment -> rendering.render(environment) } 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 6bbb6c7b..a67cf1c9 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 @@ -43,7 +43,7 @@ import kotlin.reflect.KClass * * ``` * // Function references to @Composable functions aren't supported yet. - * val FooBinding = bindCompose { showFoo(it) } + * val FooBinding = composedViewFactory { showFoo(it) } * * @Composable * private fun showFoo(foo: FooRendering) { @@ -73,14 +73,14 @@ import kotlin.reflect.KClass * * ## Initializing Compose context * - * Often all the [bindCompose] factories in an app need to share some context – for example, certain + * Often all the [composedViewFactory]s in an app need to share some context – for example, certain * ambients need to be provided, such as `MaterialTheme`. To configure this shared context, include * a [ComposeViewFactoryRoot] in your top-level [ViewEnvironment] (e.g. by using - * [withComposeViewFactoryRoot]). The first time a [bindCompose] is used to show a rendering, its - * [showRendering] function will be wrapped with the [ComposeViewFactoryRoot]. See the documentation - * on [ComposeViewFactoryRoot] for more information. + * [withComposeViewFactoryRoot]). The first time a [composedViewFactory] is used to show a + * rendering, its [showRendering] function will be wrapped with the [ComposeViewFactoryRoot]. + * See the documentation on [ComposeViewFactoryRoot] for more information. */ -inline fun bindCompose( +inline fun composedViewFactory( noinline showRendering: @Composable() ( rendering: RenderingT, environment: 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 23163615..34e4667e 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 @@ -33,12 +33,12 @@ private val HasViewFactoryRootBeenApplied = staticAmbientOf { false } /** * A `@Composable` function that is stored in a [ViewEnvironment] and will be used to wrap the first - * [bindCompose] composition. This can be used to setup any ambients that all [bindCompose] - * factories need access to, such as ambients that specify the UI theme. + * [composedViewFactory] composition. This can be used to setup any ambients that all + * [composedViewFactory]s need access to, such as ambients that specify the UI theme. * - * This function will called once, to wrap the _highest-level_ [bindCompose] in the tree. However, - * ambients are propagated down to child [bindCompose] compositions, so any ambients provided here - * will be available in _all_ [bindCompose] compositions. + * This function will called once, to wrap the _highest-level_ [composedViewFactory] in the tree. + * However, ambients are propagated down to child [composedViewFactory] compositions, so any + * ambients provided here will be available in _all_ [composedViewFactory] compositions. */ interface ComposeViewFactoryRoot { @@ -51,7 +51,7 @@ interface ComposeViewFactoryRoot { /** * Adds a [ComposeViewFactoryRoot] to this [ViewEnvironment] that uses [wrapper] to wrap the first - * [bindCompose] composition. See [ComposeViewFactoryRoot] for more information. + * [composedViewFactory] composition. See [ComposeViewFactoryRoot] for more information. */ fun ViewEnvironment.withComposeViewFactoryRoot( wrapper: @Composable() (content: @Composable() () -> Unit) -> Unit diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeWorkflow.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeWorkflow.kt index 1a30bcb6..0af19c3f 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeWorkflow.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeWorkflow.kt @@ -27,7 +27,7 @@ import com.squareup.workflow.ui.compose.internal.ComposeWorkflowImpl /** * A stateless [Workflow][com.squareup.workflow.Workflow] that [renders][render] itself as * [Composable] function. Effectively defines an inline - * [bindCompose][com.squareup.workflow.ui.compose.bindCompose]. + * [composedViewFactory][com.squareup.workflow.ui.compose.composedViewFactory]. * * This workflow does not have access to a [RenderContext][com.squareup.workflow.RenderContext] * since render contexts are only valid during render passes, and this workflow's [render] method diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt index cdb618f5..aabd9c4f 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt @@ -38,7 +38,7 @@ import com.squareup.workflow.ui.compose.internal.showRendering * val child: Any * ) * - * val FramedContainerViewFactory = bindCompose { rendering, environment -> + * val FramedContainerViewFactory = composedViewFactory { rendering, environment -> * Surface(border = Border(rendering.borderColor, 8.dp)) { * environment.showRendering(rendering.child) * } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt index 633530e8..3b205fbc 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt @@ -68,11 +68,11 @@ internal fun ViewGroup.setOrSubcomposeContent( content: @Composable() () -> Unit ) { if (parentComposition != null) { - // Somewhere above us in the workflow rendering tree, there's another bindCompose factory. + // Somewhere above us in the workflow rendering tree, there's another composedViewFactory. // We need to link to its composition reference so we inherit its ambients. setContent(Recomposer.current(), parentComposition, content) } else { - // This is the first bindCompose factory in the rendering tree, so it won't be a child + // This is the first composedViewFactory 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 70048c41..db33eb55 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 @@ -38,7 +38,8 @@ import com.squareup.workflow.ui.showRendering * Renders [rendering] into the composition using the `ViewRegistry` from the [ViewEnvironment] to * determine how to draw it. * - * To display a nested rendering from a [Composable view binding][bindCompose], use + * To display a nested rendering from a + * [Composable view binding][com.squareup.workflow.ui.compose.composedViewFactory], use * [ViewEnvironment.showRendering]. * * *Note: [rendering] must be the same type as this [ViewFactory], even though the type system does diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt index 80f6f2aa..15511c93 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt @@ -25,7 +25,7 @@ import com.squareup.workflow.ui.ViewRegistry /** * Renders [rendering] into the composition using this [ViewRegistry] to determine how to draw it. * - * To display a nested rendering from a [Composable view binding][bindCompose], use + * To display a nested rendering from a [Composable view binding][composedViewFactory], use * [ViewEnvironment.showRendering]. * * @see ViewEnvironment.showRendering diff --git a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt index 1f06ed11..f51613ab 100644 --- a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt +++ b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt @@ -25,10 +25,10 @@ import androidx.ui.layout.wrapContentSize import androidx.ui.material.ripple.ripple import androidx.ui.tooling.preview.Preview import com.squareup.sample.hellocomposebinding.HelloWorkflow.Rendering -import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.composedViewFactory import com.squareup.workflow.ui.compose.tooling.preview -val HelloBinding = bindCompose { rendering, _ -> +val HelloBinding = composedViewFactory { rendering, _ -> Clickable( modifier = Modifier.fillMaxSize() .ripple(bounded = true), diff --git a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt index 277c0590..57d19a87 100644 --- a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt +++ b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt @@ -23,9 +23,9 @@ import androidx.ui.layout.fillMaxSize import androidx.ui.layout.wrapContentSize import androidx.ui.material.ripple.ripple import com.squareup.sample.hellocompose.HelloWorkflow.Rendering -import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.composedViewFactory -val HelloBinding = bindCompose { rendering, _ -> +val HelloBinding = composedViewFactory { rendering, _ -> Clickable( onClick = { rendering.onClick() }, modifier = Modifier.ripple(bounded = true) diff --git a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt index 48bbd88d..54b078a2 100644 --- a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt +++ b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt @@ -39,7 +39,7 @@ import androidx.ui.res.dimensionResource import androidx.ui.tooling.preview.Preview import com.squareup.sample.nestedrenderings.RecursiveWorkflow.Rendering import com.squareup.workflow.ui.ViewEnvironment -import com.squareup.workflow.ui.compose.bindCompose +import com.squareup.workflow.ui.compose.composedViewFactory import com.squareup.workflow.ui.compose.showRendering import com.squareup.workflow.ui.compose.tooling.preview @@ -51,7 +51,7 @@ val BackgroundColorAmbient = ambientOf { error("No background color speci /** * A `ViewFactory` that renders [RecursiveWorkflow.Rendering]s. */ -val RecursiveViewFactory = bindCompose { rendering, viewEnvironment -> +val RecursiveViewFactory = composedViewFactory { rendering, viewEnvironment -> // Every child should be drawn with a slightly-darker background color. val color = BackgroundColorAmbient.current val childColor = remember(color) { From 49228bdba9ffbeb666f63b24f9e3f27c572a4d66 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Tue, 19 May 2020 17:13:11 -0700 Subject: [PATCH 11/15] Rename showRendering to WorkflowRendering and make it not an extension function. Fixes #21, see that issue for rationale. --- .../tooling/PreviewComposeWorkflowTest.kt | 10 ++--- .../compose/tooling/PreviewViewFactoryTest.kt | 21 +++++----- .../compose/tooling/PreviewViewEnvironment.kt | 2 +- .../ui/compose/tooling/ViewFactories.kt | 4 +- core-compose/api/core-compose.api | 4 +- ...istriesTest.kt => ViewEnvironmentsTest.kt} | 13 +++--- .../ui/compose/internal/ViewFactoriesTest.kt | 6 +-- .../workflow/ui/compose/ComposeViewFactory.kt | 4 +- .../workflow/ui/compose/ViewEnvironments.kt | 15 ++++--- .../ui/compose/internal/ParentComposition.kt | 4 +- .../ui/compose/internal/ViewFactories.kt | 15 +++---- .../ui/compose/internal/ViewRegistries.kt | 42 ------------------- .../com/squareup/sample/hellocompose/App.kt | 5 ++- .../nestedrenderings/RecursiveViewFactory.kt | 5 ++- 14 files changed, 56 insertions(+), 94 deletions(-) rename core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/{internal/ViewRegistriesTest.kt => ViewEnvironmentsTest.kt} (78%) delete mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt diff --git a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewComposeWorkflowTest.kt b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewComposeWorkflowTest.kt index 8b9b542e..1c09b994 100644 --- a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewComposeWorkflowTest.kt +++ b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewComposeWorkflowTest.kt @@ -31,9 +31,9 @@ import androidx.ui.test.findByText import androidx.ui.tooling.preview.Preview import androidx.ui.unit.dp import com.squareup.workflow.Workflow -import com.squareup.workflow.ui.compose.composed import com.squareup.workflow.ui.ViewEnvironmentKey -import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.WorkflowRendering +import com.squareup.workflow.ui.compose.composed import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -98,7 +98,7 @@ class PreviewComposeWorkflowTest { Column { Text(props.first) Semantics(container = true, mergeAllDescendants = true) { - environment.showRendering(rendering = props.second) + WorkflowRendering(props.second, environment) } } } @@ -111,11 +111,11 @@ class PreviewComposeWorkflowTest { Workflow.composed, Nothing> { props, _, environment -> Column { Semantics(container = true) { - environment.showRendering(rendering = props.first) + WorkflowRendering(rendering = props.first, viewEnvironment = environment) } Text(props.second) Semantics(container = true) { - environment.showRendering(rendering = props.third) + WorkflowRendering(rendering = props.third, viewEnvironment = environment) } } } diff --git a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt index b09ff3bd..1d4b44f3 100644 --- a/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt +++ b/compose-tooling/src/androidTest/java/com/squareup/workflow/ui/compose/tooling/PreviewViewFactoryTest.kt @@ -31,8 +31,8 @@ import androidx.ui.test.findByText import androidx.ui.tooling.preview.Preview import androidx.ui.unit.dp import com.squareup.workflow.ui.ViewEnvironmentKey +import com.squareup.workflow.ui.compose.WorkflowRendering import com.squareup.workflow.ui.compose.composedViewFactory -import com.squareup.workflow.ui.compose.showRendering import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -99,14 +99,15 @@ class PreviewViewFactoryTest { findByText("foo").assertIsDisplayed() } - private val ParentWithOneChild = composedViewFactory> { rendering, environment -> - Column { - Text(rendering.first) - Semantics(container = true, mergeAllDescendants = true) { - environment.showRendering(rendering = rendering.second) + private val ParentWithOneChild = + composedViewFactory> { rendering, environment -> + Column { + Text(rendering.first) + Semantics(container = true, mergeAllDescendants = true) { + WorkflowRendering(rendering.second, environment) + } } } - } @Preview @Composable private fun ParentWithOneChildPreview() { ParentWithOneChild.preview(Pair("one", "two")) @@ -116,11 +117,11 @@ class PreviewViewFactoryTest { composedViewFactory> { rendering, environment -> Column { Semantics(container = true) { - environment.showRendering(rendering = rendering.first) + WorkflowRendering(rendering.first, environment) } Text(rendering.second) Semantics(container = true) { - environment.showRendering(rendering = rendering.third) + WorkflowRendering(rendering.third, environment) } } } @@ -139,7 +140,7 @@ class PreviewViewFactoryTest { Text(rendering.text) rendering.child?.let { child -> Semantics(container = true) { - environment.showRendering(rendering = child) + WorkflowRendering(rendering = child, viewEnvironment = environment) } } } diff --git a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PreviewViewEnvironment.kt b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PreviewViewEnvironment.kt index 05a86cf8..7693e8b5 100644 --- a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PreviewViewEnvironment.kt +++ b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PreviewViewEnvironment.kt @@ -50,7 +50,7 @@ import kotlin.reflect.KClass /** * A [ViewRegistry] that uses [mainFactory] for rendering [RenderingT]s, and [placeholderFactory] - * for all other [showRendering][com.squareup.workflow.ui.compose.showRendering] calls. + * for all other [WorkflowRendering][com.squareup.workflow.ui.compose.WorkflowRendering] calls. */ @Immutable private class PreviewViewRegistry( diff --git a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/ViewFactories.kt b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/ViewFactories.kt index 4314486f..fe9f55a2 100644 --- a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/ViewFactories.kt +++ b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/ViewFactories.kt @@ -22,7 +22,7 @@ import androidx.ui.core.Modifier import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewFactory import com.squareup.workflow.ui.ViewRegistry -import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.WorkflowRendering /** * Draws this [ViewFactory] using a special preview [ViewRegistry]. @@ -48,5 +48,5 @@ import com.squareup.workflow.ui.compose.showRendering ) { val previewEnvironment = previewViewEnvironment(placeholderModifier, viewEnvironmentUpdater, mainFactory = this) - previewEnvironment.showRendering(rendering, modifier) + WorkflowRendering(rendering, previewEnvironment, modifier) } diff --git a/core-compose/api/core-compose.api b/core-compose/api/core-compose.api index 9e593426..a2a16ed7 100644 --- a/core-compose/api/core-compose.api +++ b/core-compose/api/core-compose.api @@ -44,8 +44,8 @@ public final class com/squareup/workflow/ui/compose/ComposeWorkflowKt { } public final class com/squareup/workflow/ui/compose/ViewEnvironmentsKt { - public static final fun showRendering (Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;)V - public static synthetic fun showRendering$default (Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun WorkflowRendering (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;)V + public static synthetic fun WorkflowRendering$default (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;ILjava/lang/Object;)V } public final class com/squareup/workflow/ui/compose/WorkflowContainerKt { diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ViewEnvironmentsTest.kt similarity index 78% rename from core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt rename to core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ViewEnvironmentsTest.kt index f6302b12..364e3679 100644 --- a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewRegistriesTest.kt +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/ViewEnvironmentsTest.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.internal +package com.squareup.workflow.ui.compose import androidx.compose.FrameManager import androidx.compose.mutableStateOf @@ -24,27 +24,26 @@ 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 org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @RunWith(AndroidJUnit4::class) -class ViewRegistriesTest { +class ViewEnvironmentsTest { @Rule @JvmField val composeRule = createComposeRule() - @Test fun showRendering_recomposes_whenFactoryChanged() { - val registry1 = ViewRegistry(bindCompose { rendering, _ -> + @Test fun workflowRendering_recomposes_whenFactoryChanged() { + val registry1 = ViewRegistry(composedViewFactory { rendering, _ -> Text(rendering) }) - val registry2 = ViewRegistry(bindCompose { rendering, _ -> + val registry2 = ViewRegistry(composedViewFactory { rendering, _ -> Text(rendering.reversed()) }) val registry = mutableStateOf(registry1) composeRule.setContent { - registry.value.showRendering("hello", ViewEnvironment(registry.value)) + WorkflowRendering("hello", ViewEnvironment(registry.value)) } findByText("hello").assertIsDisplayed() 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 index 110a76b5..998e6d75 100644 --- 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 @@ -24,7 +24,7 @@ import androidx.ui.test.findByText import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.compose.composedViewFactory -import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.WorkflowRendering import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot import org.junit.Rule import org.junit.Test @@ -35,7 +35,7 @@ class ViewFactoriesTest { @Rule @JvmField val composeRule = createComposeRule() - @Test fun showRendering_wrapsFactoryWithRoot_whenAlreadyInComposition() { + @Test fun WorkflowRendering_wrapsFactoryWithRoot_whenAlreadyInComposition() { val viewEnvironment = ViewEnvironment(ViewRegistry(TestFactory)) .withComposeViewFactoryRoot { content -> Column { @@ -45,7 +45,7 @@ class ViewFactoriesTest { } composeRule.setContent { - viewEnvironment.showRendering(TestRendering("two")) + WorkflowRendering(TestRendering("two"), viewEnvironment) } findByText("one\ntwo").assertIsDisplayed() 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 a67cf1c9..bc8fc805 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 @@ -64,8 +64,8 @@ import kotlin.reflect.KClass * renderings using the [ViewRegistry][com.squareup.workflow.ui.ViewRegistry]. * * View factories defined using this function may also show nested renderings. Doing so is as simple - * as calling [ViewEnvironment.showRendering] and passing in the nested rendering. See the kdoc on - * that function for an example. + * as calling [WorkflowRendering] and passing in the nested rendering. See the kdoc on that function + * for an example. * * Nested renderings will have access to any ambients defined in outer composable, even if there are * legacy views in between them, as long as the [ViewEnvironment] is propagated continuously between diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt index aabd9c4f..d9bb433c 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewEnvironments.kt @@ -20,7 +20,7 @@ import androidx.compose.remember import androidx.ui.core.Modifier import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry -import com.squareup.workflow.ui.compose.internal.showRendering +import com.squareup.workflow.ui.compose.internal.WorkflowRendering /** * Renders [rendering] into the composition using this [ViewEnvironment]'s @@ -40,7 +40,7 @@ import com.squareup.workflow.ui.compose.internal.showRendering * * val FramedContainerViewFactory = composedViewFactory { rendering, environment -> * Surface(border = Border(rendering.borderColor, 8.dp)) { - * environment.showRendering(rendering.child) + * WorkflowRendering(rendering.child, environment) * } * } * ``` @@ -52,10 +52,15 @@ import com.squareup.workflow.ui.compose.internal.showRendering * * @throws IllegalArgumentException if no factory can be found for [rendering]'s type. */ -@Composable fun ViewEnvironment.showRendering( +@Composable fun WorkflowRendering( rendering: Any, + viewEnvironment: ViewEnvironment, modifier: Modifier = Modifier ) { - val viewRegistry = remember(this) { this[ViewRegistry] } - viewRegistry.showRendering(rendering, this, modifier) + val viewRegistry = remember(viewEnvironment) { viewEnvironment[ViewRegistry] } + val renderingType = rendering::class + val viewFactory = remember(viewRegistry, renderingType) { + viewRegistry.getFactoryFor(renderingType) + } + WorkflowRendering(rendering, viewFactory, viewEnvironment, modifier) } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt index 3b205fbc..454365de 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ParentComposition.kt @@ -30,9 +30,9 @@ import com.squareup.workflow.ui.ViewEnvironmentKey * Holds a [CompositionReference] and that can be passed to [setOrSubcomposeContent] to create a * composition that is a child of another composition. Subcompositions get ambients and other * compose context from their parent, and propagate invalidations, which allows ambients provided - * around a [showRendering] call to be read by nested Compose-based view factories. + * around a [WorkflowRendering] call to be read by nested Compose-based view factories. * - * When [showRendering] is called, it will store an instance of this class in the [ViewEnvironment]. + * When [WorkflowRendering] is called, it will store an instance of this class in the [ViewEnvironment]. * [ComposeViewFactory] pulls the reference out of the environment and uses it to link its * composition to the outer one. */ 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 db33eb55..dab4d885 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 @@ -35,27 +35,24 @@ import com.squareup.workflow.ui.compose.ComposeViewFactory import com.squareup.workflow.ui.showRendering /** - * Renders [rendering] into the composition using the `ViewRegistry` from the [ViewEnvironment] to - * determine how to draw it. + * Renders [rendering] into the composition using [viewFactory]. * * To display a nested rendering from a - * [Composable view binding][com.squareup.workflow.ui.compose.composedViewFactory], use - * [ViewEnvironment.showRendering]. + * [Composable view binding][com.squareup.workflow.ui.compose.composedViewFactory], use the overload + * without a [ViewFactory] parameter. * * *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). * - * @see ViewEnvironment.showRendering - * @see com.squareup.workflow.ui.ViewRegistry.showRendering + * @see com.squareup.workflow.ui.compose.WorkflowRendering */ -// TODO(https://issuetracker.google.com/issues/156527332) Should be ViewFactory -@Composable internal fun ViewFactory.showRendering( +@Composable internal fun WorkflowRendering( rendering: RenderingT, + viewFactory: ViewFactory, 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. diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt deleted file mode 100644 index 4fbd747d..00000000 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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.Composable -import androidx.compose.remember -import androidx.ui.core.Modifier -import com.squareup.workflow.ui.ViewEnvironment -import com.squareup.workflow.ui.ViewFactory -import com.squareup.workflow.ui.ViewRegistry - -/** - * Renders [rendering] into the composition using this [ViewRegistry] to determine how to draw it. - * - * To display a nested rendering from a [Composable view binding][composedViewFactory], use - * [ViewEnvironment.showRendering]. - * - * @see ViewEnvironment.showRendering - * @see ViewFactory.showRendering - */ -@Composable internal fun ViewRegistry.showRendering( - rendering: Any, - hints: ViewEnvironment, - modifier: Modifier = Modifier -) { - val renderingType = rendering::class - val viewFactory = remember(this, renderingType) { getFactoryFor(renderingType) } - viewFactory.showRendering(rendering, hints, modifier) -} diff --git a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt index c6676a24..f778326e 100644 --- a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt +++ b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt @@ -27,7 +27,7 @@ import com.squareup.workflow.diagnostic.SimpleLoggingDiagnosticListener import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.compose.WorkflowContainer -import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.WorkflowRendering private val viewRegistry = ViewRegistry(HelloBinding) private val viewEnvironment = ViewEnvironment(viewRegistry) @@ -38,8 +38,9 @@ private val viewEnvironment = ViewEnvironment(viewRegistry) diagnosticListener = SimpleLoggingDiagnosticListener() ) { rendering -> MaterialTheme { - viewEnvironment.showRendering( + WorkflowRendering( rendering, + viewEnvironment, modifier = Modifier.drawBorder( shape = RoundedCornerShape(10.dp), size = 10.dp, diff --git a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt index 54b078a2..5bbbd7d6 100644 --- a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt +++ b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveViewFactory.kt @@ -40,7 +40,7 @@ import androidx.ui.tooling.preview.Preview import com.squareup.sample.nestedrenderings.RecursiveWorkflow.Rendering import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.compose.composedViewFactory -import com.squareup.workflow.ui.compose.showRendering +import com.squareup.workflow.ui.compose.WorkflowRendering import com.squareup.workflow.ui.compose.tooling.preview /** @@ -109,10 +109,11 @@ val RecursiveViewFactory = composedViewFactory { rendering, viewEnvir horizontalGravity = CenterHorizontally ) { children.forEach { childRendering -> - viewEnvironment.showRendering( + WorkflowRendering( childRendering, // Pass a weight so all children are partitioned evenly within the total column space. // Without the weight, each child is the full size of the parent. + viewEnvironment, modifier = Modifier.weight(1f, true) .padding(dimensionResource(R.dimen.recursive_padding)) ) From ea57ae0e05556aacdea52034d1f5af38f0ac6394 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Mon, 18 May 2020 13:32:27 -0700 Subject: [PATCH 12/15] Renames ComposeViewFactoryRoot to CompositionRoot and decouples the implementation. The root is now applied via a `ViewRegistry` wrapper that wraps individual factories to apply the root, instead of having this logic hard-coded inside `ComposeViewFactory`. This approach is more flexible in general (could be used to do other tricks), and decouples the rooting feature from the rest of the code. --- core-compose/api/core-compose.api | 24 +-- .../ui/compose/ComposeViewFactoryTest.kt | 2 +- ...toryRootTest.kt => CompositionRootTest.kt} | 112 ++++++++++++-- .../internal/ComposeViewFactoryRootTest.kt | 142 ------------------ .../ui/compose/internal/ViewFactoriesTest.kt | 4 +- .../workflow/ui/compose/ComposeViewFactory.kt | 28 +--- .../ui/compose/ComposeViewFactoryRoot.kt | 100 ------------ .../workflow/ui/compose/CompositionRoot.kt | 110 ++++++++++++++ .../internal/SafeComposeViewFactoryRoot.kt | 42 ------ .../ui/compose/internal/ViewFactories.kt | 2 +- .../ui/compose/internal/ViewRegistries.kt | 42 ++++++ .../HelloBindingActivity.kt | 4 +- .../NestedRenderingsActivity.kt | 4 +- 13 files changed, 273 insertions(+), 343 deletions(-) rename core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/{internal/SafeComposeViewFactoryRootTest.kt => CompositionRootTest.kt} (50%) delete mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt delete mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt create mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/CompositionRoot.kt delete mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRoot.kt create mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt diff --git a/core-compose/api/core-compose.api b/core-compose/api/core-compose.api index a2a16ed7..594f3674 100644 --- a/core-compose/api/core-compose.api +++ b/core-compose/api/core-compose.api @@ -15,24 +15,6 @@ public final class com/squareup/workflow/ui/compose/ComposeViewFactory : com/squ public fun getType ()Lkotlin/reflect/KClass; } -public abstract interface class com/squareup/workflow/ui/compose/ComposeViewFactoryRoot { - public static final field Companion Lcom/squareup/workflow/ui/compose/ComposeViewFactoryRoot$Companion; - public static fun ()V - public abstract fun wrap (Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;)V -} - -public final class com/squareup/workflow/ui/compose/ComposeViewFactoryRoot$Companion : com/squareup/workflow/ui/ViewEnvironmentKey { - public static final fun ()V - public fun getDefault ()Lcom/squareup/workflow/ui/compose/ComposeViewFactoryRoot; - public synthetic fun getDefault ()Ljava/lang/Object; -} - -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; -} - public abstract class com/squareup/workflow/ui/compose/ComposeWorkflow : com/squareup/workflow/Workflow { public fun ()V public fun asStatefulWorkflow ()Lcom/squareup/workflow/StatefulWorkflow; @@ -43,6 +25,12 @@ public final class com/squareup/workflow/ui/compose/ComposeWorkflowKt { public static final fun composed (Lcom/squareup/workflow/Workflow$Companion;Lkotlin/jvm/functions/Function4;)Lcom/squareup/workflow/ui/compose/ComposeWorkflow; } +public final class com/squareup/workflow/ui/compose/CompositionRootKt { + public static final fun ()V + public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewEnvironment; + public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewRegistry;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewRegistry; +} + public final class com/squareup/workflow/ui/compose/ViewEnvironmentsKt { public static final fun WorkflowRendering (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;)V public static synthetic fun WorkflowRendering$default (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;ILjava/lang/Object;)V 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 index af6a87c8..27f84d26 100644 --- 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 @@ -40,7 +40,7 @@ class ComposeViewFactoryTest { @Test fun wrapsFactoryWithRoot() { val wrapperText = mutableStateOf("one") val viewEnvironment = ViewEnvironment(ViewRegistry(TestFactory)) - .withComposeViewFactoryRoot { content -> + .withCompositionRoot { content -> Column { Text(wrapperText.value) content() diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/CompositionRootTest.kt similarity index 50% rename from core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt rename to core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/CompositionRootTest.kt index 794caad8..a900a105 100644 --- a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRootTest.kt +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/CompositionRootTest.kt @@ -13,8 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.squareup.workflow.ui.compose.internal +package com.squareup.workflow.ui.compose +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 @@ -23,28 +25,114 @@ 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.ComposeViewFactoryRoot import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import kotlin.test.assertFailsWith @RunWith(AndroidJUnit4::class) -class SafeComposeViewFactoryRootTest { +class CompositionRootTest { @Rule @JvmField val composeRule = createComposeRule() + @Test fun wrapWithRootIfNecessary_wrapsWhenNecessary() { + val root: CompositionRoot = { content -> + Column { + Text("one") + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(root) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_onlyWrapsOnce() { + val root: CompositionRoot = { content -> + Column { + Text("one") + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(root) { + Text("two") + wrapWithRootIfNecessary(root) { + Text("three") + } + } + } + + findByText("one\ntwo\nthree").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_seesUpdatesFromRootWrapper() { + val wrapperText = mutableStateOf("one") + val root: CompositionRoot = { content -> + Column { + Text(wrapperText.value) + content() + } + } + + composeRule.setContent { + wrapWithRootIfNecessary(root) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + FrameManager.framed { + wrapperText.value = "ENO" + } + findByText("ENO\ntwo").assertIsDisplayed() + } + + @Test fun wrapWithRootIfNecessary_rewrapsWhenDifferentRoot() { + val root1: CompositionRoot = { content -> + Column { + Text("one") + content() + } + } + val root2: CompositionRoot = { content -> + Column { + Text("ENO") + content() + } + } + val viewEnvironment = mutableStateOf(root1) + + composeRule.setContent { + wrapWithRootIfNecessary(viewEnvironment.value) { + Text("two") + } + } + + findByText("one\ntwo").assertIsDisplayed() + FrameManager.framed { + viewEnvironment.value = root2 + } + findByText("ENO\ntwo").assertIsDisplayed() + } + @Test fun safeComposeViewFactoryRoot_wraps_content() { - val wrapped = ComposeViewFactoryRoot { content -> + val wrapped: CompositionRoot = { content -> Column { Text("Parent") content() } } - val safeRoot = SafeComposeViewFactoryRoot(wrapped) + val safeRoot = safeCompositionRoot(wrapped) composeRule.setContent { - safeRoot.wrap { + safeRoot { // Need an explicit semantics container, otherwise both Texts will be merged into a single // Semantics object with the text "Parent\nChild". Semantics(container = true) { @@ -58,12 +146,12 @@ class SafeComposeViewFactoryRootTest { } @Test fun safeComposeViewFactoryRoot_throws_whenChildrenNotInvoked() { - val wrapped = ComposeViewFactoryRoot { } - val safeRoot = SafeComposeViewFactoryRoot(wrapped) + val wrapped: CompositionRoot = { } + val safeRoot = safeCompositionRoot(wrapped) val error = assertFailsWith { composeRule.setContent { - safeRoot.wrap {} + safeRoot {} } } @@ -74,15 +162,15 @@ class SafeComposeViewFactoryRootTest { } @Test fun safeComposeViewFactoryRoot_throws_whenChildrenInvokedMultipleTimes() { - val wrapped = ComposeViewFactoryRoot { children -> + val wrapped: CompositionRoot = { children -> children() children() } - val safeRoot = SafeComposeViewFactoryRoot(wrapped) + val safeRoot = safeCompositionRoot(wrapped) val error = assertFailsWith { composeRule.setContent { - safeRoot.wrap { + safeRoot { Text("Hello") } } 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 deleted file mode 100644 index 453c2545..00000000 --- a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ComposeViewFactoryRootTest.kt +++ /dev/null @@ -1,142 +0,0 @@ -/* - * 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/internal/ViewFactoriesTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/internal/ViewFactoriesTest.kt index 998e6d75..f88fe99c 100644 --- 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 @@ -25,7 +25,7 @@ import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.compose.composedViewFactory import com.squareup.workflow.ui.compose.WorkflowRendering -import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot +import com.squareup.workflow.ui.compose.withCompositionRoot import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -37,7 +37,7 @@ class ViewFactoriesTest { @Test fun WorkflowRendering_wrapsFactoryWithRoot_whenAlreadyInComposition() { val viewEnvironment = ViewEnvironment(ViewRegistry(TestFactory)) - .withComposeViewFactoryRoot { content -> + .withCompositionRoot { content -> Column { Text("one") content() 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 bc8fc805..648ab3f8 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 @@ -73,12 +73,11 @@ import kotlin.reflect.KClass * * ## Initializing Compose context * - * Often all the [composedViewFactory]s in an app need to share some context – for example, certain - * ambients need to be provided, such as `MaterialTheme`. To configure this shared context, include - * a [ComposeViewFactoryRoot] in your top-level [ViewEnvironment] (e.g. by using - * [withComposeViewFactoryRoot]). The first time a [composedViewFactory] is used to show a - * rendering, its [showRendering] function will be wrapped with the [ComposeViewFactoryRoot]. - * See the documentation on [ComposeViewFactoryRoot] for more information. + * Often all the [composedViewFactory] factories in an app need to share some context – for example, + * certain ambients need to be provided, such as `MaterialTheme`. To configure this shared context, + * call [withCompositionRoot] on your top-level [ViewEnvironment]. The first time a + * [composedViewFactory] is used to show a rendering, its [showRendering] function will be wrapped + * with the [CompositionRoot]. See the documentation on [CompositionRoot] for more information. */ inline fun composedViewFactory( noinline showRendering: @Composable() ( @@ -90,7 +89,7 @@ inline fun composedViewFactory( @PublishedApi internal class ComposeViewFactory( override val type: KClass, - private val content: @Composable() (RenderingT, ViewEnvironment) -> Unit + internal val content: @Composable() (RenderingT, ViewEnvironment) -> Unit ) : ViewFactory { override fun buildView( @@ -132,22 +131,9 @@ internal class ComposeViewFactory( val parentComposition = initialViewEnvironment[ParentComposition] composeContainer.setOrSubcomposeContent(parentComposition.reference) { val (rendering, environment) = renderState.value!! - showRenderingWrappedWithRoot(rendering, environment) + content(rendering, environment) } return composeContainer } - - /** - * Invokes [content]. If this is the highest [ComposeViewFactory] in the tree, wraps with - * the [ComposeViewFactoryRoot] if present in the [ViewEnvironment]. - */ - @Composable internal fun showRenderingWrappedWithRoot( - rendering: RenderingT, - viewEnvironment: ViewEnvironment - ) { - 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 deleted file mode 100644 index 34e4667e..00000000 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ComposeViewFactoryRoot.kt +++ /dev/null @@ -1,100 +0,0 @@ -/* - * 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 - -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 - * [composedViewFactory] composition. This can be used to setup any ambients that all - * [composedViewFactory]s need access to, such as ambients that specify the UI theme. - * - * This function will called once, to wrap the _highest-level_ [composedViewFactory] in the tree. - * However, ambients are propagated down to child [composedViewFactory] compositions, so any - * ambients provided here will be available in _all_ [composedViewFactory] compositions. - */ -interface ComposeViewFactoryRoot { - - @Composable fun wrap(content: @Composable() () -> Unit) - - companion object : ViewEnvironmentKey(ComposeViewFactoryRoot::class) { - override val default: ComposeViewFactoryRoot get() = NoopComposeViewFactoryRoot - } -} - -/** - * Adds a [ComposeViewFactoryRoot] to this [ViewEnvironment] that uses [wrapper] to wrap the first - * [composedViewFactory] composition. See [ComposeViewFactoryRoot] for more information. - */ -fun ViewEnvironment.withComposeViewFactoryRoot( - wrapper: @Composable() (content: @Composable() () -> Unit) -> Unit -): ViewEnvironment = this + (ComposeViewFactoryRoot to ComposeViewFactoryRoot(wrapper)) - -// This could be inline, but that makes the Compose compiler puke. -@Suppress("FunctionName") -fun ComposeViewFactoryRoot( - wrapper: @Composable() (content: @Composable() () -> Unit) -> Unit -): ComposeViewFactoryRoot = object : 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/CompositionRoot.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/CompositionRoot.kt new file mode 100644 index 00000000..b01dc6fa --- /dev/null +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/CompositionRoot.kt @@ -0,0 +1,110 @@ +/* + * 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 + +import androidx.annotation.VisibleForTesting +import androidx.annotation.VisibleForTesting.PRIVATE +import androidx.compose.Composable +import androidx.compose.Providers +import androidx.compose.remember +import androidx.compose.staticAmbientOf +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.compose.internal.mapFactories +import kotlin.reflect.KClass + +/** + * Used by [wrapWithRootIfNecessary] to ensure the [CompositionRoot] is only applied once. + */ +private val HasViewFactoryRootBeenApplied = staticAmbientOf { false } + +/** + * A `@Composable` function that will be used to wrap the first (highest-level) + * [composedViewFactory] view factory in a composition. This can be used to setup any ambients that + * all [composedViewFactory] factories need access to, such as e.g. UI themes. + * + * This function will called once, to wrap the _highest-level_ [composedViewFactory] in the tree. + * However, ambients are propagated down to child [composedViewFactory] compositions, so any + * ambients provided here will be available in _all_ [composedViewFactory] compositions. + */ +typealias CompositionRoot = @Composable() (content: @Composable() () -> Unit) -> Unit + +/** + * Convenience function for applying a [CompositionRoot] to this [ViewEnvironment]'s [ViewRegistry]. + * See [ViewRegistry.withCompositionRoot]. + */ +fun ViewEnvironment.withCompositionRoot(root: CompositionRoot): ViewEnvironment = + this + (ViewRegistry to this[ViewRegistry].withCompositionRoot(root)) + +/** + * Returns a [ViewRegistry] that ensures that any [composedViewFactory] factories registered in this + * registry will be wrapped exactly once with a [CompositionRoot] wrapper. + * See [CompositionRoot] for more information. + */ +fun ViewRegistry.withCompositionRoot(root: CompositionRoot): ViewRegistry = + mapFactories { factory -> + if (factory !is ComposeViewFactory) return@mapFactories factory + + @Suppress("UNCHECKED_CAST") + ComposeViewFactory(factory.type as KClass) { rendering, environment -> + wrapWithRootIfNecessary(root) { + (factory as ComposeViewFactory).content(rendering, environment) + } + } + } + +/** + * Adds [content] to the composition, ensuring that [CompositionRoot] has been applied. Will only + * wrap the content at the highest occurrence of this function in the composition subtree. + */ +@VisibleForTesting(otherwise = PRIVATE) +@Composable internal fun wrapWithRootIfNecessary( + root: CompositionRoot, + 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 safeRoot: CompositionRoot = remember(root) { safeCompositionRoot(root) } + safeRoot(content) + } + } +} + +/** + * [CompositionRoot] that asserts that the content method invokes its children parameter + * exactly once, and throws an [IllegalStateException] if not. + */ +internal fun safeCompositionRoot(delegate: CompositionRoot): CompositionRoot = { content -> + var childrenCalledCount = 0 + delegate { + childrenCalledCount++ + content() + } + check(childrenCalledCount == 1) { + "Expected ComposableDecorator to invoke children exactly once, " + + "but was invoked $childrenCalledCount times." + } +} diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRoot.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRoot.kt deleted file mode 100644 index c2460be2..00000000 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/SafeComposeViewFactoryRoot.kt +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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 androidx.compose.Composable -import com.squareup.workflow.ui.compose.ComposeViewFactoryRoot - -/** - * [ComposeViewFactoryRoot] that asserts that the [wrap] method invokes its children parameter - * exactly once, and throws an [IllegalStateException] if not. - */ -internal class SafeComposeViewFactoryRoot( - private val delegate: ComposeViewFactoryRoot -) : ComposeViewFactoryRoot { - - @Composable override fun wrap(content: @Composable() () -> Unit) { - var childrenCalledCount = 0 - delegate.wrap { - childrenCalledCount++ - content() - } - check(childrenCalledCount == 1) { - "Expected ComposableDecorator to invoke children exactly once, " + - "but was invoked $childrenCalledCount times." - } - } -} 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 dab4d885..4e1a7fee 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 @@ -57,7 +57,7 @@ import com.squareup.workflow.ui.showRendering // "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 (viewFactory is ComposeViewFactory) { - viewFactory.showRenderingWrappedWithRoot(rendering, viewEnvironment) + viewFactory.content(rendering, viewEnvironment) return@Box } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt new file mode 100644 index 00000000..8bec77e0 --- /dev/null +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/internal/ViewRegistries.kt @@ -0,0 +1,42 @@ +/* + * 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 com.squareup.workflow.ui.ViewFactory +import com.squareup.workflow.ui.ViewRegistry +import kotlin.reflect.KClass + +/** + * Applies [transform] to each [ViewFactory] in this registry. Transformations are applied lazily, + * at the time of lookup via [ViewRegistry.getFactoryFor]. + */ +internal fun ViewRegistry.mapFactories( + transform: (ViewFactory<*>) -> ViewFactory<*> +): ViewRegistry = object : ViewRegistry { + override val keys: Set> get() = this@mapFactories.keys + + override fun getFactoryFor( + renderingType: KClass + ): ViewFactory { + val transformedFactory = transform(this@mapFactories.getFactoryFor(renderingType)) + check(transformedFactory.type == renderingType) { + "Expected transform to return a ViewFactory that is compatible with $renderingType, " + + "but got one with type ${transformedFactory.type}" + } + @Suppress("UNCHECKED_CAST") + return transformedFactory as ViewFactory + } +} diff --git a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBindingActivity.kt b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBindingActivity.kt index 47971e35..adcb5db4 100644 --- a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBindingActivity.kt +++ b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBindingActivity.kt @@ -22,11 +22,11 @@ import com.squareup.workflow.diagnostic.SimpleLoggingDiagnosticListener import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.WorkflowRunner -import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot +import com.squareup.workflow.ui.compose.withCompositionRoot import com.squareup.workflow.ui.setContentWorkflow private val viewRegistry = ViewRegistry(HelloBinding) -private val containerHints = ViewEnvironment(viewRegistry).withComposeViewFactoryRoot { content -> +private val containerHints = ViewEnvironment(viewRegistry).withCompositionRoot { content -> MaterialTheme(content = content) } diff --git a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt index bd4d021d..210d9df2 100644 --- a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt +++ b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt @@ -23,7 +23,7 @@ import com.squareup.workflow.diagnostic.SimpleLoggingDiagnosticListener import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.WorkflowRunner -import com.squareup.workflow.ui.compose.withComposeViewFactoryRoot +import com.squareup.workflow.ui.compose.withCompositionRoot import com.squareup.workflow.ui.setContentWorkflow private val viewRegistry = ViewRegistry( @@ -31,7 +31,7 @@ private val viewRegistry = ViewRegistry( LegacyRunner ) -private val viewEnvironment = ViewEnvironment(viewRegistry).withComposeViewFactoryRoot { content -> +private val viewEnvironment = ViewEnvironment(viewRegistry).withCompositionRoot { content -> Providers(BackgroundColorAmbient provides Color.Green, children = content) } From 4a5373a60882bb3a7164d0cadaeae92ece232fe0 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Tue, 19 May 2020 15:35:07 -0700 Subject: [PATCH 13/15] Make renderAsState public, make WorkflowContainer take a ViewEnvironment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This narrows the scope of `WorkflowContainer` to be only for showing a workflow's renderings using a `ViewEnvironment`. This elimnates the need to have separate overloads for workflows with rendering type `ComposeRendering`, and is more idiomatic – the old `WorkflowContainer` wasn't a "container" in any Compose-y sense of the word, and it was really weird that it took a content function. Now this composable is actually a container for all workflow-related composables, and so I think the name is actually appropriate (closes #22). To render a workflow without a `ViewEnvironment`, `renderAsState` is now public. This is also much more idiomatic, as it resembles APIs like `Flow.collectAsState` and `Observable.subscribeAsState`. --- core-compose/api/core-compose.api | 31 +- .../workflow/ui/compose/RenderAsStateTest.kt | 234 ++++++++++++++ .../ui/compose/WorkflowContainerTest.kt | 169 +--------- .../workflow/ui/compose/RenderAsState.kt | 244 +++++++++++++++ .../workflow/ui/compose/WorkflowContainer.kt | 289 ++++-------------- .../com/squareup/sample/hellocompose/App.kt | 26 +- 6 files changed, 570 insertions(+), 423 deletions(-) create mode 100644 core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/RenderAsStateTest.kt create mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/RenderAsState.kt diff --git a/core-compose/api/core-compose.api b/core-compose/api/core-compose.api index 594f3674..90273676 100644 --- a/core-compose/api/core-compose.api +++ b/core-compose/api/core-compose.api @@ -31,28 +31,31 @@ public final class com/squareup/workflow/ui/compose/CompositionRootKt { public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewRegistry;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewRegistry; } +public final class com/squareup/workflow/ui/compose/RenderAsStateKt { + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; + public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; + public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; + public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; + public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; +} + public final class com/squareup/workflow/ui/compose/ViewEnvironmentsKt { public static final fun WorkflowRendering (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;)V public static synthetic fun WorkflowRendering$default (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;ILjava/lang/Object;)V } public final class com/squareup/workflow/ui/compose/WorkflowContainerKt { - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;)V public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Lkotlin/jvm/functions/Function2;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V } public final class com/squareup/workflow/ui/compose/internal/ComposeSupportKt { diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/RenderAsStateTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/RenderAsStateTest.kt new file mode 100644 index 00000000..868c6fe7 --- /dev/null +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/RenderAsStateTest.kt @@ -0,0 +1,234 @@ +/* + * 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 + +import androidx.compose.FrameManager +import androidx.compose.Providers +import androidx.compose.mutableStateOf +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.ui.savedinstancestate.UiSavedStateRegistry +import androidx.ui.savedinstancestate.UiSavedStateRegistryAmbient +import androidx.ui.test.createComposeRule +import androidx.ui.test.runOnIdleCompose +import androidx.ui.test.waitForIdle +import com.google.common.truth.Truth.assertThat +import com.squareup.workflow.RenderContext +import com.squareup.workflow.Snapshot +import com.squareup.workflow.StatefulWorkflow +import com.squareup.workflow.Workflow +import com.squareup.workflow.action +import com.squareup.workflow.parse +import com.squareup.workflow.readUtf8WithLength +import com.squareup.workflow.stateless +import com.squareup.workflow.ui.compose.RenderAsStateTest.SnapshottingWorkflow.SnapshottedRendering +import com.squareup.workflow.writeUtf8WithLength +import okio.ByteString +import okio.ByteString.Companion.decodeBase64 +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class RenderAsStateTest { + + @Rule @JvmField val composeRule = createComposeRule() + + @Test fun passesPropsThrough() { + val workflow = Workflow.stateless { it } + lateinit var initialRendering: String + + composeRule.setContent { + initialRendering = workflow.renderAsState("foo").value + } + + runOnIdleCompose { + assertThat(initialRendering).isEqualTo("foo") + } + } + + @Test fun seesPropsAndRenderingUpdates() { + val workflow = Workflow.stateless { it } + val props = mutableStateOf("foo") + lateinit var rendering: String + + composeRule.setContent { + rendering = workflow.renderAsState(props.value).value + } + + waitForIdle() + assertThat(rendering).isEqualTo("foo") + FrameManager.framed { + props.value = "bar" + } + waitForIdle() + assertThat(rendering).isEqualTo("bar") + } + + @Test fun invokesOutputCallback() { + val workflow = Workflow.stateless Unit> { + { string -> actionSink.send(action { setOutput(string) }) } + } + val receivedOutputs = mutableListOf() + lateinit var rendering: (String) -> Unit + + composeRule.setContent { + rendering = workflow.renderAsState(onOutput = { receivedOutputs += it }).value + } + + waitForIdle() + assertThat(receivedOutputs).isEmpty() + rendering("one") + + waitForIdle() + assertThat(receivedOutputs).isEqualTo(listOf("one")) + rendering("two") + + waitForIdle() + assertThat(receivedOutputs).isEqualTo(listOf("one", "two")) + } + + @Test fun savesSnapshot() { + val workflow = SnapshottingWorkflow() + val savedStateRegistry = UiSavedStateRegistry(emptyMap()) { true } + lateinit var rendering: SnapshottedRendering + + composeRule.setContent { + Providers(UiSavedStateRegistryAmbient provides savedStateRegistry) { + rendering = renderAsStateImpl( + workflow, + props = Unit, + onOutput = {}, + diagnosticListener = null, + snapshotKey = SNAPSHOT_KEY + ).value + } + } + + waitForIdle() + assertThat(rendering.string).isEmpty() + rendering.updateString("foo") + + waitForIdle() + val savedValues = FrameManager.framed { + savedStateRegistry.performSave() + } + println("saved keys: ${savedValues.keys}") + // Relying on the int key across all runtimes is brittle, so use an explicit key. + val snapshot = ByteString.of(*(savedValues.getValue(SNAPSHOT_KEY) as ByteArray)) + println("snapshot: ${snapshot.base64()}") + assertThat(snapshot).isEqualTo(EXPECTED_SNAPSHOT) + } + + @Test fun restoresSnapshot() { + val workflow = SnapshottingWorkflow() + val restoreValues = mapOf(SNAPSHOT_KEY to EXPECTED_SNAPSHOT.toByteArray()) + val savedStateRegistry = UiSavedStateRegistry(restoreValues) { true } + lateinit var rendering: SnapshottedRendering + + composeRule.setContent { + Providers(UiSavedStateRegistryAmbient provides savedStateRegistry) { + rendering = renderAsStateImpl( + workflow, + props = Unit, + onOutput = {}, + diagnosticListener = null, + snapshotKey = "workflow-snapshot" + ).value + } + } + + waitForIdle() + assertThat(rendering.string).isEqualTo("foo") + } + + @Test fun restoresFromSnapshotWhenWorkflowChanged() { + val workflow1 = SnapshottingWorkflow() + val workflow2 = SnapshottingWorkflow() + val currentWorkflow = mutableStateOf(workflow1) + lateinit var rendering: SnapshottedRendering + + var compositionCount = 0 + var lastCompositionCount = 0 + fun assertWasRecomposed() { + assertThat(compositionCount).isGreaterThan(lastCompositionCount) + lastCompositionCount = compositionCount + } + + composeRule.setContent { + compositionCount++ + rendering = currentWorkflow.value.renderAsState().value + } + + // Initialize the first workflow. + waitForIdle() + assertThat(rendering.string).isEmpty() + assertWasRecomposed() + rendering.updateString("one") + waitForIdle() + assertWasRecomposed() + assertThat(rendering.string).isEqualTo("one") + + // Change the workflow instance being rendered. This should restart the runtime, but restore + // it from the snapshot. + FrameManager.framed { + currentWorkflow.value = workflow2 + } + + waitForIdle() + assertWasRecomposed() + assertThat(rendering.string).isEqualTo("one") + } + + private companion object { + const val SNAPSHOT_KEY = "workflow-snapshot" + + /** Golden value from [savesSnapshot]. */ + val EXPECTED_SNAPSHOT = "AAAABwAAAANmb28AAAAA".decodeBase64()!! + } + + // Seems to be a problem accessing Workflow.stateful. + private class SnapshottingWorkflow : + StatefulWorkflow() { + + data class SnapshottedRendering( + val string: String, + val updateString: (String) -> Unit + ) + + override fun initialState( + props: Unit, + snapshot: Snapshot? + ): String = snapshot?.bytes?.parse { it.readUtf8WithLength() } ?: "" + + override fun render( + props: Unit, + state: String, + context: RenderContext + ) = SnapshottedRendering( + string = state, + updateString = { newString -> context.actionSink.send(updateString(newString)) } + ) + + override fun snapshotState(state: String): Snapshot = + Snapshot.write { it.writeUtf8WithLength(state) } + + private fun updateString(newString: String) = action { + nextState = newString + } + } +} diff --git a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/WorkflowContainerTest.kt b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/WorkflowContainerTest.kt index 716b9375..8ae3f540 100644 --- a/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/WorkflowContainerTest.kt +++ b/core-compose/src/androidTest/java/com/squareup/workflow/ui/compose/WorkflowContainerTest.kt @@ -17,33 +17,15 @@ package com.squareup.workflow.ui.compose -import androidx.compose.FrameManager -import androidx.compose.Providers -import androidx.compose.mutableStateOf -import androidx.compose.onActive import androidx.test.ext.junit.runners.AndroidJUnit4 -import androidx.ui.foundation.Clickable import androidx.ui.foundation.Text -import androidx.ui.layout.Column -import androidx.ui.savedinstancestate.UiSavedStateRegistry -import androidx.ui.savedinstancestate.UiSavedStateRegistryAmbient +import androidx.ui.test.assertIsDisplayed import androidx.ui.test.createComposeRule -import androidx.ui.test.doClick import androidx.ui.test.findByText -import androidx.ui.test.waitForIdle -import com.google.common.truth.Truth.assertThat -import com.squareup.workflow.RenderContext -import com.squareup.workflow.Snapshot -import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Workflow -import com.squareup.workflow.action -import com.squareup.workflow.parse -import com.squareup.workflow.readUtf8WithLength import com.squareup.workflow.stateless -import com.squareup.workflow.ui.compose.WorkflowContainerTest.SnapshottingWorkflow.SnapshottedRendering -import com.squareup.workflow.writeUtf8WithLength -import okio.ByteString -import okio.ByteString.Companion.decodeBase64 +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewRegistry import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -53,150 +35,27 @@ class WorkflowContainerTest { @Rule @JvmField val composeRule = createComposeRule() - @Test fun passesPropsThrough() { - val workflow = Workflow.stateless { it } + @Test fun rendersFromViewRegistry() { + val workflow = Workflow.stateless { "hello" } + val registry = ViewRegistry(composedViewFactory { rendering, _ -> Text(rendering) }) composeRule.setContent { - WorkflowContainer(workflow, "foo") { - assertThat(it).isEqualTo("foo") - } - } - } - - @Test fun seesPropsAndRenderingUpdates() { - val workflow = Workflow.stateless { it } - val props = mutableStateOf("foo") - - composeRule.setContent { - WorkflowContainer(workflow, props.value) { - Text(it) - } + WorkflowContainer(workflow, ViewEnvironment(registry)) } - findByText("foo").assertExists() - FrameManager.framed { - props.value = "bar" - } - findByText("bar").assertExists() + findByText("hello").assertIsDisplayed() } - @Test fun invokesOutputCallback() { - val workflow = Workflow.stateless Unit> { - { string -> actionSink.send(action { setOutput(string) }) } - } - - val receivedOutputs = mutableListOf() - composeRule.setContent { - WorkflowContainer(workflow, onOutput = { receivedOutputs += it }) { sendOutput -> - Column { - Clickable(onClick = { sendOutput("one") }) { - Text("send one") - } - Clickable(onClick = { sendOutput("two") }) { - Text("send two") - } - } - } + @Test fun automaticallyAddsComposeRenderingFactory() { + val workflow = Workflow.composed { _, _, _ -> + Text("it worked") } - - waitForIdle() - assertThat(receivedOutputs).isEmpty() - findByText("send one").doClick() - - waitForIdle() - assertThat(receivedOutputs).isEqualTo(listOf("one")) - findByText("send two").doClick() - - waitForIdle() - assertThat(receivedOutputs).isEqualTo(listOf("one", "two")) - } - - @Test fun savesSnapshot() { - val savedStateRegistry = UiSavedStateRegistry(emptyMap()) { true } + val registry = ViewRegistry() composeRule.setContent { - Providers(UiSavedStateRegistryAmbient provides savedStateRegistry) { - WorkflowContainerImpl( - SnapshottingWorkflow, - props = Unit, - onOutput = {}, - snapshotKey = SNAPSHOT_KEY - ) { (string, updateString) -> - onActive { - assertThat(string).isEmpty() - updateString("foo") - } - } - } + WorkflowContainer(workflow, ViewEnvironment(registry)) } - waitForIdle() - val savedValues = FrameManager.framed { - savedStateRegistry.performSave() - } - println("saved keys: ${savedValues.keys}") - // Relying on the int key across all runtimes might be flaky, might need to pass explicit key. - val snapshot = ByteString.of(*(savedValues.getValue(SNAPSHOT_KEY) as ByteArray)) - println("snapshot: ${snapshot.base64()}") - assertThat(snapshot).isEqualTo(EXPECTED_SNAPSHOT) - } - - @Test fun restoresSnapshot() { - val restoreValues = mapOf(SNAPSHOT_KEY to EXPECTED_SNAPSHOT.toByteArray()) - val savedStateRegistry = UiSavedStateRegistry(restoreValues) { true } - - composeRule.setContent { - Providers(UiSavedStateRegistryAmbient provides savedStateRegistry) { - WorkflowContainerImpl( - SnapshottingWorkflow, - props = Unit, - onOutput = {}, - snapshotKey = "workflow-snapshot" - ) { (string) -> - onActive { - assertThat(string).isEqualTo("foo") - } - Text(string) - } - } - } - - findByText("foo").assertExists() - } - - private companion object { - const val SNAPSHOT_KEY = "workflow-snapshot" - val EXPECTED_SNAPSHOT = "AAAABwAAAANmb28AAAAA".decodeBase64()!! - } - - // Seems to be a problem accessing Workflow.stateful. - private object SnapshottingWorkflow : - StatefulWorkflow() { - - data class SnapshottedRendering( - val string: String, - val updateString: (String) -> Unit - ) - - override fun initialState( - props: Unit, - snapshot: Snapshot? - ): String = snapshot?.bytes?.parse { it.readUtf8WithLength() } ?: "" - - override fun render( - props: Unit, - state: String, - context: RenderContext - ) = SnapshottedRendering( - string = state, - updateString = { newString -> context.actionSink.send(updateString(newString)) } - ) - - override fun snapshotState(state: String): Snapshot = - Snapshot.write { it.writeUtf8WithLength(state) } - - private fun updateString(newString: String) = action { - nextState = newString - } + findByText("it worked").assertIsDisplayed() } } diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/RenderAsState.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/RenderAsState.kt new file mode 100644 index 00000000..3307a3aa --- /dev/null +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/RenderAsState.kt @@ -0,0 +1,244 @@ +/* + * 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("NOTHING_TO_INLINE") + +package com.squareup.workflow.ui.compose + +import androidx.annotation.VisibleForTesting +import androidx.compose.Composable +import androidx.compose.CompositionLifecycleObserver +import androidx.compose.FrameManager +import androidx.compose.MutableState +import androidx.compose.State +import androidx.compose.mutableStateOf +import androidx.compose.remember +import androidx.ui.core.CoroutineContextAmbient +import androidx.ui.core.Ref +import androidx.ui.savedinstancestate.Saver +import androidx.ui.savedinstancestate.SaverScope +import androidx.ui.savedinstancestate.UiSavedStateRegistryAmbient +import androidx.ui.savedinstancestate.savedInstanceState +import com.squareup.workflow.Snapshot +import com.squareup.workflow.Workflow +import com.squareup.workflow.diagnostic.WorkflowDiagnosticListener +import com.squareup.workflow.launchWorkflowIn +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancel +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.consumeAsFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import okio.ByteString +import kotlin.coroutines.CoroutineContext + +/** + * Runs this [Workflow] as long as this composable is part of the composition, and returns a + * [State] object that will be updated whenever the runtime emits a new [RenderingT]. + * + * The workflow runtime will be started when this function is first added to the composition, and + * cancelled when it is removed. The first rendering will be available immediately as soon as this + * function returns, as [State.value]. Composables that read this value will automatically recompose + * whenever the runtime emits a new rendering. + * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * + * @receiver The [Workflow] to run. If the value of the receiver changes to a different [Workflow] + * while this function is in the composition, the runtime will be restarted with the new workflow. + * @param props The [PropsT] for the root [Workflow]. Changes to this value across different + * compositions will cause the root workflow to re-render with the new props. + * @param onOutput A function that will be executed whenever the root [Workflow] emits an output. + * @param diagnosticListener An optional [WorkflowDiagnosticListener] to start the runtime with. If + * this value changes while this function is in the composition, the runtime will be restarted. + */ +@Composable +fun Workflow.renderAsState( + props: PropsT, + onOutput: (OutputT) -> Unit, + diagnosticListener: WorkflowDiagnosticListener? = null +): State = renderAsStateImpl(this, props, onOutput, diagnosticListener) + +/** + * Runs this [Workflow] as long as this composable is part of the composition, and returns a + * [State] object that will be updated whenever the runtime emits a new [RenderingT]. + * + * The workflow runtime will be started when this function is first added to the composition, and + * cancelled when it is removed. The first rendering will be available immediately as soon as this + * function returns, as [State.value]. Composables that read this value will automatically recompose + * whenever the runtime emits a new rendering. + * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * + * @receiver The [Workflow] to run. If the value of the receiver changes to a different [Workflow] + * while this function is in the composition, the runtime will be restarted with the new workflow. + * @param onOutput A function that will be executed whenever the root [Workflow] emits an output. + * @param diagnosticListener An optional [WorkflowDiagnosticListener] to start the runtime with. If + * this value changes while this function is in the composition, the runtime will be restarted. + */ +@Composable +inline fun Workflow.renderAsState( + noinline onOutput: (OutputT) -> Unit, + diagnosticListener: WorkflowDiagnosticListener? = null +): State = renderAsState(Unit, onOutput, diagnosticListener) + +/** + * Runs this [Workflow] as long as this composable is part of the composition, and returns a + * [State] object that will be updated whenever the runtime emits a new [RenderingT]. + * + * The workflow runtime will be started when this function is first added to the composition, and + * cancelled when it is removed. The first rendering will be available immediately as soon as this + * function returns, as [State.value]. Composables that read this value will automatically recompose + * whenever the runtime emits a new rendering. + * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * + * @receiver The [Workflow] to run. If the value of the receiver changes to a different [Workflow] + * while this function is in the composition, the runtime will be restarted with the new workflow. + * @param props The [PropsT] for the root [Workflow]. Changes to this value across different + * compositions will cause the root workflow to re-render with the new props. + * @param diagnosticListener An optional [WorkflowDiagnosticListener] to start the runtime with. If + * this value changes while this function is in the composition, the runtime will be restarted. + */ +@Composable +inline fun Workflow.renderAsState( + props: PropsT, + diagnosticListener: WorkflowDiagnosticListener? = null +): State = renderAsState(props, {}, diagnosticListener) + +/** + * Runs this [Workflow] as long as this composable is part of the composition, and returns a + * [State] object that will be updated whenever the runtime emits a new [RenderingT]. + * + * The workflow runtime will be started when this function is first added to the composition, and + * cancelled when it is removed. The first rendering will be available immediately as soon as this + * function returns, as [State.value]. Composables that read this value will automatically recompose + * whenever the runtime emits a new rendering. + * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * + * @receiver The [Workflow] to run. If the value of the receiver changes to a different [Workflow] + * while this function is in the composition, the runtime will be restarted with the new workflow. + * @param diagnosticListener An optional [WorkflowDiagnosticListener] to start the runtime with. If + * this value changes while this function is in the composition, the runtime will be restarted. + */ +@Composable +inline fun Workflow.renderAsState( + diagnosticListener: WorkflowDiagnosticListener? = null +): State = renderAsState(Unit, {}, diagnosticListener) + +/** + * @param snapshotKey Allows tests to pass in a custom key to use to save/restore the snapshot from + * the [UiSavedStateRegistryAmbient]. If null, will use the default key based on source location. + */ +@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) +@Composable internal fun renderAsStateImpl( + workflow: Workflow, + props: PropsT, + onOutput: (OutputT) -> Unit, + diagnosticListener: WorkflowDiagnosticListener?, + snapshotKey: String? = null +): State { + @Suppress("DEPRECATION") + val coroutineContext = CoroutineContextAmbient.current + Dispatchers.Main.immediate + val snapshotState = savedInstanceState(key = snapshotKey, saver = SnapshotSaver) { null } + + val outputRef = remember { Ref<(OutputT) -> Unit>() } + outputRef.value = onOutput + + // We can't use onActive/on(Pre)Commit because they won't run their callback until after this + // function returns, and we need to run this immediately so we get the rendering synchronously. + val state = remember(coroutineContext, workflow, diagnosticListener) { + WorkflowState(coroutineContext, workflow, props, outputRef, snapshotState, diagnosticListener) + } + state.setProps(props) + + return state.rendering +} + +@Suppress("EXPERIMENTAL_API_USAGE") +private class WorkflowState( + coroutineContext: CoroutineContext, + workflow: Workflow, + initialProps: PropsT, + private val outputRef: Ref<(OutputT) -> Unit>, + private val snapshotState: MutableState, + private val diagnosticListener: WorkflowDiagnosticListener? +) : CompositionLifecycleObserver { + + private val workflowScope = CoroutineScope(coroutineContext) + private val renderingState = mutableStateOf(null) + + // This can be a StateFlow once coroutines is upgraded to 1.3.6. + private val propsChannel = Channel(capacity = Channel.CONFLATED) + .apply { offer(initialProps) } + val propsFlow = propsChannel.consumeAsFlow() + .distinctUntilChanged() + + // The value is guaranteed to be set before returning, so this cast is fine. + @Suppress("UNCHECKED_CAST") + val rendering: State + get() = renderingState as State + + init { + launchWorkflowIn(workflowScope, workflow, propsFlow, snapshotState.value) { session -> + session.diagnosticListener = diagnosticListener + + session.outputs.onEach { outputRef.value!!.invoke(it) } + .launchIn(this) + + session.renderingsAndSnapshots + .onEach { (rendering, snapshot) -> + FrameManager.framed { + renderingState.value = rendering + snapshotState.value = snapshot + } + } + .launchIn(this) + } + } + + fun setProps(props: PropsT) { + propsChannel.offer(props) + } + + override fun onEnter() {} + + override fun onLeave() { + workflowScope.cancel() + } +} + +private object SnapshotSaver : Saver { + override fun SaverScope.save(value: Snapshot?): ByteArray { + return value?.bytes?.toByteArray() ?: ByteArray(0) + } + + override fun restore(value: ByteArray): Snapshot? { + return value.takeUnless { it.isEmpty() } + ?.let { bytes -> Snapshot.of(ByteString.of(*bytes)) } + } +} + +private class OutputCallback(var onOutput: (OutputT) -> Unit) diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/WorkflowContainer.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/WorkflowContainer.kt index 643af54f..908864a6 100644 --- a/core-compose/src/main/java/com/squareup/workflow/ui/compose/WorkflowContainer.kt +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/WorkflowContainer.kt @@ -14,46 +14,22 @@ * limitations under the License. */ @file:Suppress( - "EXPERIMENTAL_API_USAGE", "FunctionNaming", - "NOTHING_TO_INLINE", - "RemoveEmptyParenthesesFromAnnotationEntry" + "NOTHING_TO_INLINE" ) package com.squareup.workflow.ui.compose -import androidx.annotation.VisibleForTesting -import androidx.annotation.VisibleForTesting.PRIVATE import androidx.compose.Composable -import androidx.compose.Direct -import androidx.compose.Pivotal -import androidx.compose.State -import androidx.compose.onDispose import androidx.compose.remember -import androidx.compose.state -import androidx.ui.core.CoroutineContextAmbient import androidx.ui.core.Modifier -import androidx.ui.foundation.Box -import androidx.ui.savedinstancestate.Saver -import androidx.ui.savedinstancestate.SaverScope -import androidx.ui.savedinstancestate.UiSavedStateRegistryAmbient -import androidx.ui.savedinstancestate.savedInstanceState import com.squareup.workflow.Snapshot import com.squareup.workflow.Workflow import com.squareup.workflow.diagnostic.WorkflowDiagnosticListener -import com.squareup.workflow.launchWorkflowIn import com.squareup.workflow.ui.ViewEnvironment -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.cancel -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.channels.Channel.Factory.CONFLATED -import kotlinx.coroutines.flow.consumeAsFlow -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach -import okio.ByteString -import kotlin.coroutines.CoroutineContext +import com.squareup.workflow.ui.ViewFactory +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.plus /** * Render a [Workflow]'s renderings. @@ -62,121 +38,33 @@ import kotlin.coroutines.CoroutineContext * any time [workflow], [diagnosticListener], or the `CoroutineContext` * changes. The runtime will be cancelled when this function stops composing. * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * * @param workflow The [Workflow] to render. * @param props The props to render the root workflow with. If this value changes between calls, * the workflow runtime will re-render with the new props. * @param onOutput A function that will be invoked any time the root workflow emits an output. + * @param viewEnvironment The [ViewEnvironment] used to display renderings. + * @param modifier The [Modifier] to apply to the root [ViewFactory]. * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. - * @param content A [Composable] function that gets executed every time the root workflow spits - * out a new rendering. */ -@Direct -@Composable fun WorkflowContainer( +@Composable fun WorkflowContainer( workflow: Workflow, props: PropsT, onOutput: (OutputT) -> Unit, - modifier: Modifier = Modifier, - diagnosticListener: WorkflowDiagnosticListener? = null, - content: @Composable() (rendering: RenderingT) -> Unit -) { - WorkflowContainerImpl(workflow, props, onOutput, modifier, diagnosticListener, content = content) -} - -/** - * Render a [Workflow]'s renderings. - * - * When this function is first composed it will start a new runtime. This runtime will be restarted - * any time [workflow], [diagnosticListener], or the `CoroutineContext` - * changes. The runtime will be cancelled when this function stops composing. - * - * @param workflow The [Workflow] to render. - * @param onOutput A function that will be invoked any time the root workflow emits an output. - * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. - * @param content A [Composable] function that gets executed every time the root workflow spits - * out a new rendering. - */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, - noinline onOutput: (OutputT) -> Unit, - modifier: Modifier = Modifier, - diagnosticListener: WorkflowDiagnosticListener? = null, - noinline content: @Composable() (rendering: RenderingT) -> Unit -) { - WorkflowContainer(workflow, Unit, onOutput, modifier, diagnosticListener, content) -} - -/** - * Render a [Workflow]'s renderings. - * - * When this function is first composed it will start a new runtime. This runtime will be restarted - * any time [workflow], [diagnosticListener], or the `CoroutineContext` - * changes. The runtime will be cancelled when this function stops composing. - * - * @param workflow The [Workflow] to render. - * @param props The props to render the root workflow with. If this value changes between calls, - * the workflow runtime will re-render with the new props. - * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. - * @param content A [Composable] function that gets executed every time the root workflow spits - * out a new rendering. - */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, - props: PropsT, - modifier: Modifier = Modifier, - diagnosticListener: WorkflowDiagnosticListener? = null, - noinline content: @Composable() (rendering: RenderingT) -> Unit -) { - WorkflowContainer(workflow, props, {}, modifier, diagnosticListener, content) -} - -/** - * Render a [Workflow]'s renderings. - * - * When this function is first composed it will start a new runtime. This runtime will be restarted - * any time [workflow], [diagnosticListener], or the `CoroutineContext` - * changes. The runtime will be cancelled when this function stops composing. - * - * @param workflow The [Workflow] to render. - * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. - * @param content A [Composable] function that gets executed every time the root workflow spits - * out a new rendering. - */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, - modifier: Modifier = Modifier, - diagnosticListener: WorkflowDiagnosticListener? = null, - noinline content: @Composable() (rendering: RenderingT) -> Unit -) { - WorkflowContainer(workflow, Unit, {}, modifier, diagnosticListener, content) -} - -/** - * Render a [Workflow]'s renderings. - * - * When this function is first composed it will start a new runtime. This runtime will be restarted - * any time [workflow], [diagnosticListener], or the `CoroutineContext` - * changes. The runtime will be cancelled when this function stops composing. - * - * @param workflow The [Workflow] to render. - * @param viewEnvironment The [ViewEnvironment] used to show the [ComposeRendering]s emitted by - * the workflow. - * @param props The props to render the root workflow with. If this value changes between calls, - * the workflow runtime will re-render with the new props. - * @param onOutput A function that will be invoked any time the root workflow emits an output. - * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. - */ -@Direct -@Composable fun WorkflowContainer( - workflow: Workflow, viewEnvironment: ViewEnvironment, - props: PropsT, - onOutput: (OutputT) -> Unit, modifier: Modifier = Modifier, diagnosticListener: WorkflowDiagnosticListener? = null ) { - WorkflowContainer(workflow, props, onOutput, modifier, diagnosticListener) { rendering -> - rendering.render(viewEnvironment) + // Ensure ComposeRendering is in the ViewRegistry. + val realEnvironment = remember(viewEnvironment) { + viewEnvironment.withFactory(ComposeRendering.Factory) } + + val rendering = workflow.renderAsState(props, onOutput, diagnosticListener) + WorkflowRendering(rendering.value, realEnvironment, modifier) } /** @@ -186,20 +74,24 @@ import kotlin.coroutines.CoroutineContext * any time [workflow], [diagnosticListener], or the `CoroutineContext` * changes. The runtime will be cancelled when this function stops composing. * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * * @param workflow The [Workflow] to render. - * @param viewEnvironment The [ViewEnvironment] used to show the [ComposeRendering]s emitted by - * the workflow. * @param onOutput A function that will be invoked any time the root workflow emits an output. + * @param viewEnvironment The [ViewEnvironment] used to display renderings. + * @param modifier The [Modifier] to apply to the root [ViewFactory]. * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, - viewEnvironment: ViewEnvironment, +@Composable inline fun WorkflowContainer( + workflow: Workflow, noinline onOutput: (OutputT) -> Unit, + viewEnvironment: ViewEnvironment, modifier: Modifier = Modifier, diagnosticListener: WorkflowDiagnosticListener? = null ) { - WorkflowContainer(workflow, viewEnvironment, Unit, onOutput, modifier, diagnosticListener) + WorkflowContainer(workflow, Unit, onOutput, viewEnvironment, modifier, diagnosticListener) } /** @@ -209,21 +101,25 @@ import kotlin.coroutines.CoroutineContext * any time [workflow], [diagnosticListener], or the `CoroutineContext` * changes. The runtime will be cancelled when this function stops composing. * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * * @param workflow The [Workflow] to render. - * @param viewEnvironment The [ViewEnvironment] used to show the [ComposeRendering]s emitted by - * the workflow. * @param props The props to render the root workflow with. If this value changes between calls, * the workflow runtime will re-render with the new props. + * @param viewEnvironment The [ViewEnvironment] used to display renderings. + * @param modifier The [Modifier] to apply to the root [ViewFactory]. * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, - viewEnvironment: ViewEnvironment, +@Composable inline fun WorkflowContainer( + workflow: Workflow, props: PropsT, + viewEnvironment: ViewEnvironment, modifier: Modifier = Modifier, diagnosticListener: WorkflowDiagnosticListener? = null ) { - WorkflowContainer(workflow, viewEnvironment, props, {}, modifier, diagnosticListener) + WorkflowContainer(workflow, props, {}, viewEnvironment, modifier, diagnosticListener) } /** @@ -233,111 +129,28 @@ import kotlin.coroutines.CoroutineContext * any time [workflow], [diagnosticListener], or the `CoroutineContext` * changes. The runtime will be cancelled when this function stops composing. * + * [Snapshot]s from the runtime will automatically be saved to the current + * [UiSavedStateRegistry][androidx.ui.savedinstancestate.UiSavedStateRegistry]. When the runtime is + * started, if a snapshot exists in the registry, it will be used to restore the workflows. + * * @param workflow The [Workflow] to render. - * @param viewEnvironment The [ViewEnvironment] used to show the [ComposeRendering]s emitted by - * the workflow. + * @param viewEnvironment The [ViewEnvironment] used to display renderings. + * @param modifier The [Modifier] to apply to the root [ViewFactory]. * @param diagnosticListener A [WorkflowDiagnosticListener] to configure on the runtime. */ -@Composable inline fun WorkflowContainer( - workflow: Workflow, +@Composable inline fun WorkflowContainer( + workflow: Workflow, viewEnvironment: ViewEnvironment, modifier: Modifier = Modifier, diagnosticListener: WorkflowDiagnosticListener? = null ) { - WorkflowContainer(workflow, viewEnvironment, Unit, {}, modifier, diagnosticListener) -} - -/** - * Internal version of [WorkflowContainer] that accepts extra parameters for testing. - */ -@VisibleForTesting(otherwise = PRIVATE) -@Composable internal fun WorkflowContainerImpl( - workflow: Workflow, - props: PropsT, - onOutput: (OutputT) -> Unit, - modifier: Modifier = Modifier, - diagnosticListener: WorkflowDiagnosticListener? = null, - snapshotKey: String? = null, - content: @Composable() (rendering: RenderingT) -> Unit -) { - @Suppress("DEPRECATION") - val rendering = renderAsState( - workflow, props, onOutput, CoroutineContextAmbient.current, diagnosticListener, snapshotKey - ) - - Box(modifier = modifier) { - content(rendering.value) - } -} - -/** - * @param snapshotKey Allows tests to pass in a custom key to use to save/restore the snapshot from - * the [UiSavedStateRegistryAmbient]. If null, will use the default key based on source location. - */ -@Composable private fun renderAsState( - @Pivotal workflow: Workflow, - props: PropsT, - onOutput: (OutputT) -> Unit, - @Pivotal coroutineContext: CoroutineContext, - @Pivotal diagnosticListener: WorkflowDiagnosticListener?, - snapshotKey: String? -): State { - // This can be a StateFlow once coroutines is upgraded to 1.3.6. - val propsChannel = remember { Channel(capacity = CONFLATED) } - propsChannel.offer(props) - - // Need a mutable holder for onOutput so the outputs subscriber created in the onActive block - // will always be able to see the latest value. - val outputCallback = remember { OutputCallback(onOutput) } - outputCallback.onOutput = onOutput - - val renderingState = state { null } - val snapshotState = savedInstanceState(key = snapshotKey, saver = SnapshotSaver) { null } - - // We can't use onActive/on(Pre)Commit because they won't run their callback until after this - // function returns, and we need to run this immediately so we get the rendering synchronously. - val workflowScope = remember { - val coroutineScope = CoroutineScope(coroutineContext + Dispatchers.Main.immediate) - val propsFlow = propsChannel.consumeAsFlow() - .distinctUntilChanged() - - launchWorkflowIn(coroutineScope, workflow, propsFlow, snapshotState.value) { session -> - session.diagnosticListener = diagnosticListener - - // Don't call onOutput directly, since out captured reference won't be changed if the - // if a different argument is passed to observeWorkflow. - session.outputs.onEach { outputCallback.onOutput(it) } - .launchIn(this) - - session.renderingsAndSnapshots - .onEach { (rendering, snapshot) -> - renderingState.value = rendering - snapshotState.value = snapshot - } - .launchIn(this) - } - - return@remember coroutineScope - } - - onDispose { - workflowScope.cancel() - } - - // The value is guaranteed to be set before returning, so this cast is fine. - @Suppress("UNCHECKED_CAST") - return renderingState as State + WorkflowContainer(workflow, Unit, {}, viewEnvironment, modifier, diagnosticListener) } -private object SnapshotSaver : Saver { - override fun SaverScope.save(value: Snapshot?): ByteArray { - return value?.bytes?.toByteArray() ?: ByteArray(0) - } - - override fun restore(value: ByteArray): Snapshot? { - return value.takeUnless { it.isEmpty() } - ?.let { bytes -> Snapshot.of(ByteString.of(*bytes)) } +private fun ViewEnvironment.withFactory(viewFactory: ViewFactory<*>): ViewEnvironment { + return this[ViewRegistry].let { registry -> + if (viewFactory.type !in registry.keys) { + this + (ViewRegistry to registry + viewFactory) + } else this } } - -private class OutputCallback(var onOutput: (OutputT) -> Unit) diff --git a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt index f778326e..da73f861 100644 --- a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt +++ b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/App.kt @@ -27,27 +27,21 @@ import com.squareup.workflow.diagnostic.SimpleLoggingDiagnosticListener import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry import com.squareup.workflow.ui.compose.WorkflowContainer -import com.squareup.workflow.ui.compose.WorkflowRendering private val viewRegistry = ViewRegistry(HelloBinding) private val viewEnvironment = ViewEnvironment(viewRegistry) @Composable fun App() { - WorkflowContainer( - workflow = HelloWorkflow, - diagnosticListener = SimpleLoggingDiagnosticListener() - ) { rendering -> - MaterialTheme { - WorkflowRendering( - rendering, - viewEnvironment, - modifier = Modifier.drawBorder( - shape = RoundedCornerShape(10.dp), - size = 10.dp, - color = Color.Magenta - ) - ) - } + MaterialTheme { + WorkflowContainer( + HelloWorkflow, viewEnvironment, + modifier = Modifier.drawBorder( + shape = RoundedCornerShape(10.dp), + size = 10.dp, + color = Color.Magenta + ), + diagnosticListener = SimpleLoggingDiagnosticListener() + ) } } From f54fa51093ffad32b9dff5471fc75fe286f49065 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Wed, 27 May 2020 20:38:52 -0700 Subject: [PATCH 14/15] Update Compose to dev12. --- buildSrc/src/main/java/Dependencies.kt | 2 +- compose-tooling/api/compose-tooling.api | 6 +-- .../compose/tooling/PlaceholderViewFactory.kt | 13 +++--- core-compose/api/core-compose.api | 39 +++++++----------- .../ui/compose/ComposeViewFactoryTest.kt | 7 ++-- .../ui/compose/internal/ComposeSupportTest.kt | 12 ++++-- .../ui/compose/internal/ComposeSupport.kt | 40 +++++-------------- .../hellocomposebinding/HelloBinding.kt | 14 +++---- .../HelloRenderingWorkflow.kt | 19 +++++---- .../sample/hellocompose/HelloBinding.kt | 17 ++++---- settings.gradle.kts | 4 +- 11 files changed, 73 insertions(+), 100 deletions(-) diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 9ac3f8f2..42b941cd 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -19,7 +19,7 @@ import java.util.Locale.US import kotlin.reflect.full.declaredMembers object Versions { - const val compose = "0.1.0-dev11" + const val compose = "0.1.0-dev12" const val kotlin = "1.3.71" const val targetSdk = 29 const val workflow = "0.28.0" diff --git a/compose-tooling/api/compose-tooling.api b/compose-tooling/api/compose-tooling.api index c283e9b9..6af39e71 100644 --- a/compose-tooling/api/compose-tooling.api +++ b/compose-tooling/api/compose-tooling.api @@ -6,12 +6,10 @@ public final class com/squareup/workflow/ui/compose/tooling/BuildConfig { } public final class com/squareup/workflow/ui/compose/tooling/ComposeWorkflowsKt { - public static final fun preview (Lcom/squareup/workflow/ui/compose/ComposeWorkflow;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;)V - public static synthetic fun preview$default (Lcom/squareup/workflow/ui/compose/ComposeWorkflow;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun preview (Lcom/squareup/workflow/ui/compose/ComposeWorkflow;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;III)V } public final class com/squareup/workflow/ui/compose/tooling/ViewFactoriesKt { - public static final fun preview (Lcom/squareup/workflow/ui/ViewFactory;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;)V - public static synthetic fun preview$default (Lcom/squareup/workflow/ui/ViewFactory;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun preview (Lcom/squareup/workflow/ui/ViewFactory;Ljava/lang/Object;Landroidx/ui/core/Modifier;Landroidx/ui/core/Modifier;Lkotlin/jvm/functions/Function1;Landroidx/compose/Composer;III)V } diff --git a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt index 9987b25b..5c66c40a 100644 --- a/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt +++ b/compose-tooling/src/main/java/com/squareup/workflow/ui/compose/tooling/PlaceholderViewFactory.kt @@ -18,7 +18,6 @@ package com.squareup.workflow.ui.compose.tooling import androidx.compose.Composable -import androidx.ui.core.DrawScope import androidx.ui.core.Modifier import androidx.ui.core.clipToBounds import androidx.ui.core.drawBehind @@ -26,12 +25,14 @@ import androidx.ui.foundation.Box import androidx.ui.foundation.Text import androidx.ui.foundation.drawBorder import androidx.ui.geometry.Offset +import androidx.ui.geometry.toRect import androidx.ui.graphics.Color import androidx.ui.graphics.Paint import androidx.ui.graphics.Shadow -import androidx.ui.graphics.painter.Stroke -import androidx.ui.graphics.painter.drawCanvas -import androidx.ui.graphics.painter.rotate +import androidx.ui.graphics.drawscope.DrawScope +import androidx.ui.graphics.drawscope.Stroke +import androidx.ui.graphics.drawscope.drawCanvas +import androidx.ui.graphics.drawscope.rotate import androidx.ui.graphics.withSaveLayer import androidx.ui.layout.fillMaxSize import androidx.ui.text.TextStyle @@ -39,8 +40,6 @@ import androidx.ui.text.style.TextAlign import androidx.ui.tooling.preview.Preview import androidx.ui.unit.Dp import androidx.ui.unit.dp -import androidx.ui.unit.px -import androidx.ui.unit.toRect import com.squareup.workflow.ui.ViewFactory import com.squareup.workflow.ui.compose.composedViewFactory @@ -70,7 +69,7 @@ internal fun placeholderViewFactory(modifier: Modifier): ViewFactory = style = TextStyle( textAlign = TextAlign.Center, color = Color.White, - shadow = Shadow(blurRadius = 5.px, color = Color.Black) + shadow = Shadow(blurRadius = 5f, color = Color.Black) ) ) } diff --git a/core-compose/api/core-compose.api b/core-compose/api/core-compose.api index 90273676..86dd0c51 100644 --- a/core-compose/api/core-compose.api +++ b/core-compose/api/core-compose.api @@ -1,7 +1,7 @@ public final class com/squareup/workflow/ui/compose/ComposeRendering { public static final field Companion Lcom/squareup/workflow/ui/compose/ComposeRendering$Companion; public static final fun ()V - public fun (Lkotlin/jvm/functions/Function2;)V + public fun (Lkotlin/jvm/functions/Function4;)V } public final class com/squareup/workflow/ui/compose/ComposeRendering$Companion { @@ -10,7 +10,7 @@ public final class com/squareup/workflow/ui/compose/ComposeRendering$Companion { } public final class com/squareup/workflow/ui/compose/ComposeViewFactory : com/squareup/workflow/ui/ViewFactory { - public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function3;)V + public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function5;)V public fun buildView (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View; public fun getType ()Lkotlin/reflect/KClass; } @@ -18,44 +18,35 @@ public final class com/squareup/workflow/ui/compose/ComposeViewFactory : com/squ public abstract class com/squareup/workflow/ui/compose/ComposeWorkflow : com/squareup/workflow/Workflow { public fun ()V public fun asStatefulWorkflow ()Lcom/squareup/workflow/StatefulWorkflow; - public abstract fun render (Ljava/lang/Object;Lcom/squareup/workflow/Sink;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/compose/Composer;)V + public abstract fun render (Ljava/lang/Object;Lcom/squareup/workflow/Sink;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/compose/Composer;II)V } public final class com/squareup/workflow/ui/compose/ComposeWorkflowKt { - public static final fun composed (Lcom/squareup/workflow/Workflow$Companion;Lkotlin/jvm/functions/Function4;)Lcom/squareup/workflow/ui/compose/ComposeWorkflow; + public static final fun composed (Lcom/squareup/workflow/Workflow$Companion;Lkotlin/jvm/functions/Function6;)Lcom/squareup/workflow/ui/compose/ComposeWorkflow; } public final class com/squareup/workflow/ui/compose/CompositionRootKt { public static final fun ()V - public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewEnvironment; - public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewRegistry;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/ui/ViewRegistry; + public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewEnvironment;Lkotlin/jvm/functions/Function4;)Lcom/squareup/workflow/ui/ViewEnvironment; + public static final fun withCompositionRoot (Lcom/squareup/workflow/ui/ViewRegistry;Lkotlin/jvm/functions/Function4;)Lcom/squareup/workflow/ui/ViewRegistry; } public final class com/squareup/workflow/ui/compose/RenderAsStateKt { - public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; - public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; - public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; - public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)Landroidx/compose/State; - public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; - public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; - public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; - public static synthetic fun renderAsState$default (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)Landroidx/compose/State; + public static final fun renderAsState (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)Landroidx/compose/State; } public final class com/squareup/workflow/ui/compose/ViewEnvironmentsKt { - public static final fun WorkflowRendering (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;)V - public static synthetic fun WorkflowRendering$default (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun WorkflowRendering (Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Landroidx/compose/Composer;III)V } public final class com/squareup/workflow/ui/compose/WorkflowContainerKt { - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V - public static synthetic fun WorkflowContainer$default (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;ILjava/lang/Object;)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)V + public static final fun WorkflowContainer (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/ui/ViewEnvironment;Landroidx/ui/core/Modifier;Lcom/squareup/workflow/diagnostic/WorkflowDiagnosticListener;Landroidx/compose/Composer;III)V } public final class com/squareup/workflow/ui/compose/internal/ComposeSupportKt { 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 index 27f84d26..2bd3c255 100644 --- 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 @@ -22,7 +22,6 @@ 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 @@ -53,11 +52,13 @@ class ComposeViewFactoryTest { RootView(viewEnvironment = viewEnvironment) } - findByText("one\ntwo").assertIsDisplayed() + // Compose bug doesn't let us use assertIsDisplayed on older devices. + // See https://issuetracker.google.com/issues/157728188. + findByText("one\ntwo").assertExists() FrameManager.framed { wrapperText.value = "ENO" } - findByText("ENO\ntwo").assertIsDisplayed() + findByText("ENO\ntwo").assertExists() } private class RootView(context: Context) : FrameLayout(context) { 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 index 0c6a10d1..1a23845d 100644 --- 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 @@ -30,8 +30,8 @@ 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.findBySubstring import androidx.ui.test.findByText import org.junit.Rule import org.junit.Test @@ -47,7 +47,9 @@ class ComposeSupportTest { TestComposable("foo") } - findByText("foo").assertIsDisplayed() + // Compose bug doesn't let us use assertIsDisplayed on older devices. + // See https://issuetracker.google.com/issues/157728188. + findByText("foo").assertExists() } @Test fun ambientChangesPassThroughSubcomposition() { @@ -56,11 +58,13 @@ class ComposeSupportTest { TestComposable(ambientValue.value) } - findByText("foo").assertIsDisplayed() + // Compose bug doesn't let us use assertIsDisplayed on older devices. + // See https://issuetracker.google.com/issues/157728188. + findBySubstring("foo").assertExists() FrameManager.framed { ambientValue.value = "bar" } - findByText("bar").assertIsDisplayed() + findByText("bar").assertExists() } @Composable private fun TestComposable(ambientValue: String) { 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 72c97918..f5a5b9b7 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 @@ -18,25 +18,19 @@ package com.squareup.workflow.ui.compose.internal import android.content.Context -import android.view.View import android.view.ViewGroup import androidx.compose.Composable import androidx.compose.Composition import androidx.compose.CompositionReference +import androidx.compose.FrameManager import androidx.compose.Recomposer import androidx.compose.compositionFor -import androidx.lifecycle.LifecycleOwner +import androidx.ui.core.AndroidOwner import androidx.ui.node.UiComposer -import com.squareup.workflow.ui.compose.internal.ReflectionSupport.ANDROID_OWNER_CLASS -import com.squareup.workflow.ui.compose.internal.ReflectionSupport.androidOwnerView -import com.squareup.workflow.ui.compose.internal.ReflectionSupport.createOwner import com.squareup.workflow.ui.compose.internal.ReflectionSupport.createWrappedContent -import com.squareup.workflow.ui.compose.internal.ReflectionSupport.ownerRoot import com.squareup.workflow.ui.core.compose.R -private typealias AndroidOwner = Any private typealias WrappedComposition = Composition -private typealias LayoutNode = Any private val DefaultLayoutParams = ViewGroup.LayoutParams( ViewGroup.LayoutParams.WRAP_CONTENT, @@ -59,12 +53,13 @@ internal fun ViewGroup.setContent( parent: CompositionReference, content: @Composable() () -> Unit ): Composition { + FrameManager.ensureStarted() val composeView: AndroidOwner = if (childCount > 0) { - getChildAt(0).takeIf(ANDROID_OWNER_CLASS::isInstance) + getChildAt(0) as? AndroidOwner } else { removeAllViews(); null - } ?: createOwner(context).also { addView(androidOwnerView(it), DefaultLayoutParams) } + } ?: AndroidOwner(context).also { addView(it.view, DefaultLayoutParams) } return doSetContent(context, composeView, recomposer, parent, content) } @@ -80,21 +75,20 @@ private fun doSetContent( content: @Composable() () -> Unit ): Composition { // val original = compositionFor(context, owner.root, recomposer) - val container = ownerRoot(owner) val original = compositionFor( - container = container, + container = owner.root, recomposer = recomposer, parent = parent, composerFactory = { slotTable, factoryRecomposer -> - UiComposer(context, container, slotTable, factoryRecomposer) + UiComposer(context, owner.root, slotTable, factoryRecomposer) } ) - val wrapped = androidOwnerView(owner).getTag(R.id.wrapped_composition_tag) + val wrapped = owner.view.getTag(R.id.wrapped_composition_tag) as? WrappedComposition // ?: WrappedComposition(owner, original).also { ?: createWrappedContent(owner, original).also { - androidOwnerView(owner).setTag(R.id.wrapped_composition_tag, it) + owner.view.setTag(R.id.wrapped_composition_tag, it) } wrapped.setContent(content) return wrapped @@ -102,31 +96,17 @@ private fun doSetContent( private object ReflectionSupport { - val ANDROID_OWNER_CLASS = Class.forName("androidx.ui.core.AndroidOwner") private val WRAPPED_COMPOSITION_CLASS = Class.forName("androidx.ui.core.WrappedComposition") - private val ANDROID_OWNER_KT_CLASS = Class.forName("androidx.ui.core.AndroidOwnerKt") private val WRAPPED_COMPOSITION_CTOR = - WRAPPED_COMPOSITION_CLASS.getConstructor(ANDROID_OWNER_CLASS, Composition::class.java) - - private val CREATE_OWNER_FUN = - ANDROID_OWNER_KT_CLASS.getMethod("createOwner", Context::class.java, LifecycleOwner::class.java) - private val ANDROID_OWNER_ROOT_GETTER = ANDROID_OWNER_CLASS.getMethod("getRoot") + WRAPPED_COMPOSITION_CLASS.getConstructor(AndroidOwner::class.java, Composition::class.java) init { WRAPPED_COMPOSITION_CTOR.isAccessible = true } - fun createOwner(context: Context): AndroidOwner = - CREATE_OWNER_FUN.invoke(null, context, null) as AndroidOwner - - fun ownerRoot(owner: AndroidOwner): LayoutNode = - ANDROID_OWNER_ROOT_GETTER.invoke(owner) as LayoutNode - fun createWrappedContent( owner: AndroidOwner, original: Composition ): WrappedComposition = WRAPPED_COMPOSITION_CTOR.newInstance(owner, original) as Composition - - fun androidOwnerView(owner: AndroidOwner): View = owner as View } diff --git a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt index f51613ab..a91020fa 100644 --- a/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt +++ b/samples/hello-compose-binding/src/main/java/com/squareup/sample/hellocomposebinding/HelloBinding.kt @@ -18,8 +18,8 @@ package com.squareup.sample.hellocomposebinding import androidx.compose.Composable import androidx.ui.core.Alignment import androidx.ui.core.Modifier -import androidx.ui.foundation.Clickable import androidx.ui.foundation.Text +import androidx.ui.foundation.clickable import androidx.ui.layout.fillMaxSize import androidx.ui.layout.wrapContentSize import androidx.ui.material.ripple.ripple @@ -29,13 +29,13 @@ import com.squareup.workflow.ui.compose.composedViewFactory import com.squareup.workflow.ui.compose.tooling.preview val HelloBinding = composedViewFactory { rendering, _ -> - Clickable( + Text( + rendering.message, modifier = Modifier.fillMaxSize() - .ripple(bounded = true), - onClick = { rendering.onClick() } - ) { - Text(rendering.message, modifier = Modifier.wrapContentSize(Alignment.Center)) - } + .ripple() + .clickable(onClick = rendering.onClick) + .wrapContentSize(Alignment.Center) + ) } @Preview(heightDp = 150, showBackground = true) diff --git a/samples/hello-compose-rendering/src/main/java/com/squareup/sample/hellocomposerendering/HelloRenderingWorkflow.kt b/samples/hello-compose-rendering/src/main/java/com/squareup/sample/hellocomposerendering/HelloRenderingWorkflow.kt index 1cd1906c..7faf8e71 100644 --- a/samples/hello-compose-rendering/src/main/java/com/squareup/sample/hellocomposerendering/HelloRenderingWorkflow.kt +++ b/samples/hello-compose-rendering/src/main/java/com/squareup/sample/hellocomposerendering/HelloRenderingWorkflow.kt @@ -18,17 +18,16 @@ package com.squareup.sample.hellocomposerendering import androidx.compose.Composable import androidx.ui.core.Alignment import androidx.ui.core.Modifier -import androidx.ui.foundation.Clickable import androidx.ui.foundation.Text -import androidx.ui.layout.fillMaxSize +import androidx.ui.foundation.clickable import androidx.ui.layout.wrapContentSize import androidx.ui.material.MaterialTheme import androidx.ui.material.ripple.ripple import androidx.ui.tooling.preview.Preview import com.squareup.sample.hellocomposerendering.HelloRenderingWorkflow.Toggle import com.squareup.workflow.Sink -import com.squareup.workflow.ui.compose.ComposeWorkflow import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.compose.ComposeWorkflow import com.squareup.workflow.ui.compose.tooling.preview /** @@ -46,13 +45,13 @@ object HelloRenderingWorkflow : ComposeWorkflow() { viewEnvironment: ViewEnvironment ) { MaterialTheme { - Clickable( - onClick = { outputSink.send(Toggle) }, - modifier = Modifier.ripple(bounded = true) - .fillMaxSize() - ) { - Text(props, modifier = Modifier.wrapContentSize(Alignment.Center)) - } + Text( + props, + modifier = Modifier + .ripple() + .clickable(onClick = { outputSink.send(Toggle) }) + .wrapContentSize(Alignment.Center) + ) } } } diff --git a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt index 57d19a87..5b39a449 100644 --- a/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt +++ b/samples/hello-compose/src/main/java/com/squareup/sample/hellocompose/HelloBinding.kt @@ -17,20 +17,19 @@ package com.squareup.sample.hellocompose import androidx.ui.core.Alignment import androidx.ui.core.Modifier -import androidx.ui.foundation.Clickable import androidx.ui.foundation.Text -import androidx.ui.layout.fillMaxSize +import androidx.ui.foundation.clickable import androidx.ui.layout.wrapContentSize import androidx.ui.material.ripple.ripple import com.squareup.sample.hellocompose.HelloWorkflow.Rendering import com.squareup.workflow.ui.compose.composedViewFactory val HelloBinding = composedViewFactory { rendering, _ -> - Clickable( - onClick = { rendering.onClick() }, - modifier = Modifier.ripple(bounded = true) - .fillMaxSize() - ) { - Text(rendering.message, modifier = Modifier.wrapContentSize(Alignment.Center)) - } + Text( + rendering.message, + modifier = Modifier + .ripple() + .clickable(onClick = rendering.onClick) + .wrapContentSize(Alignment.Center) + ) } diff --git a/settings.gradle.kts b/settings.gradle.kts index a273408d..7b1a1e0a 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -20,6 +20,8 @@ include( ":core-compose", ":samples:hello-compose", ":samples:hello-compose-binding", - ":samples:hello-compose-rendering", + // This module is crashing the compiler with dev12. Just disable it for now. + // See https://github.com/square/workflow-kotlin-compose/issues/42. + // ":samples:hello-compose-rendering", ":samples:nested-renderings" ) From d6a15f28884c9507b546d417e45f46549576a850 Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Sat, 16 May 2020 19:30:11 -0700 Subject: [PATCH 15/15] [DNM] Introduce ViewFactoryTransformer with vibrating demo effect. --- .../ExplodingViewFactoryTransformer.kt | 67 +++++++++++++++++++ .../ui/compose/ViewFactoryTransformer.kt | 64 ++++++++++++++++++ .../NestedRenderingsActivity.kt | 38 ++++++++--- .../nestedrenderings/RecursiveWorkflow.kt | 7 +- 4 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/ExplodingViewFactoryTransformer.kt create mode 100644 core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewFactoryTransformer.kt diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ExplodingViewFactoryTransformer.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ExplodingViewFactoryTransformer.kt new file mode 100644 index 00000000..a44202d1 --- /dev/null +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ExplodingViewFactoryTransformer.kt @@ -0,0 +1,67 @@ +package com.squareup.workflow.ui.compose + +import androidx.animation.AnimatedFloat +import androidx.animation.AnimationEndReason.TargetReached +import androidx.compose.Composable +import androidx.compose.getValue +import androidx.compose.mutableStateOf +import androidx.compose.onCommit +import androidx.compose.setValue +import androidx.ui.animation.animatedFloat +import androidx.ui.core.DensityAmbient +import androidx.ui.core.Modifier +import androidx.ui.core.drawLayer +import androidx.ui.unit.dp +import com.squareup.workflow.ui.ViewEnvironment +import kotlin.random.Random + +/** + * TODO write documentation + */ +class ExplodingViewFactoryTransformer( + min: Int = -1, + max: Int = 1 +) : ViewFactoryTransformer { + + var min by mutableStateOf(min.toFloat()) + var max by mutableStateOf(max.toFloat()) + + @Composable override fun modifyView( + renderingDepth: Int, + viewEnvironment: ViewEnvironment + ): Modifier { + val offsetXDp = animatedFloat(0f) + val offsetYDp = animatedFloat(0f) + + onCommit(min, max) { + offsetXDp.startPulseAnimation(min, max) + offsetYDp.startPulseAnimation(min, max) + } + + if (!offsetXDp.isRunning && !offsetYDp.isRunning) { + return Modifier + } + + val offsetXPx = with(DensityAmbient.current) { offsetXDp.value.dp.toPx() } + val offsetYPx = with(DensityAmbient.current) { offsetYDp.value.dp.toPx() } + return Modifier.drawLayer(translationX = offsetXPx.value, translationY = offsetYPx.value) + } + + private fun AnimatedFloat.startPulseAnimation( + min: Float, + max: Float + ) { + fun animate() { + val target = if (min == 0f && max == 0f) 0f else { + Random.nextDouble(min.toDouble(), max.toDouble()) + .toFloat() + } + animateTo(target) { reason, _ -> + if (reason == TargetReached) { + if (min != 0f || max != 0f) animate() + } + } + } + animate() + } +} diff --git a/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewFactoryTransformer.kt b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewFactoryTransformer.kt new file mode 100644 index 00000000..eaa47044 --- /dev/null +++ b/core-compose/src/main/java/com/squareup/workflow/ui/compose/ViewFactoryTransformer.kt @@ -0,0 +1,64 @@ +@file:Suppress("RemoveEmptyParenthesesFromAnnotationEntry") + +package com.squareup.workflow.ui.compose + +import androidx.compose.Composable +import androidx.compose.remember +import androidx.ui.core.Modifier +import com.squareup.workflow.ui.ViewEnvironment +import com.squareup.workflow.ui.ViewEnvironmentKey +import com.squareup.workflow.ui.ViewFactory +import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.compose.internal.showRendering +import kotlin.reflect.KClass + +/** + * TODO write documentation + */ +interface ViewFactoryTransformer { + @Composable() fun modifyView( + renderingDepth: Int, + viewEnvironment: ViewEnvironment + ): Modifier +} + +/** + * TODO kdoc + */ +fun ViewRegistry.modifyViewFactories(transformer: ViewFactoryTransformer): ViewRegistry = + TransformedViewRegistry(this, transformer) + +private class TransformedViewRegistry( + private val delegate: ViewRegistry, + private val transformer: ViewFactoryTransformer +) : ViewRegistry { + override val keys: Set> = delegate.keys + + override fun getFactoryFor( + renderingType: KClass + ): ViewFactory { + @Suppress("UNCHECKED_CAST") + val realFactory = delegate.getFactoryFor(renderingType) as ViewFactory + + @Suppress("UNCHECKED_CAST") + return ComposeViewFactory(renderingType as KClass) { rendering, environment -> + // No need to key depth on the environment, the depth will never change. + val depth = remember { environment[FactoryDepthKey] } + + val childEnvironment = remember(environment) { + environment + (FactoryDepthKey to depth + 1) + } + val modifier = transformer.modifyView(depth, environment) + realFactory.showRendering(rendering, childEnvironment, modifier) + } + } + + /** + * Values actually encode both depth and prevent infinite looping. + * Even values mean the factory should do processing, odd values mean + * direct pass-through. + */ + private object FactoryDepthKey : ViewEnvironmentKey(Int::class) { + override val default: Int get() = 0 + } +} diff --git a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt index 210d9df2..c94a1568 100644 --- a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt +++ b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/NestedRenderingsActivity.kt @@ -18,10 +18,19 @@ package com.squareup.sample.nestedrenderings import android.os.Bundle import androidx.appcompat.app.AppCompatActivity import androidx.compose.Providers +import androidx.compose.getValue +import androidx.compose.remember +import androidx.compose.setValue +import androidx.compose.state +import androidx.ui.core.setContent import androidx.ui.graphics.Color +import androidx.ui.layout.Column +import androidx.ui.material.Slider import com.squareup.workflow.diagnostic.SimpleLoggingDiagnosticListener import com.squareup.workflow.ui.ViewEnvironment import com.squareup.workflow.ui.ViewRegistry +import com.squareup.workflow.ui.compose.ExplodingViewFactoryTransformer +import com.squareup.workflow.ui.compose.WorkflowContainer import com.squareup.workflow.ui.WorkflowRunner import com.squareup.workflow.ui.compose.withCompositionRoot import com.squareup.workflow.ui.setContentWorkflow @@ -30,19 +39,32 @@ private val viewRegistry = ViewRegistry( RecursiveViewFactory, LegacyRunner ) +private val exploder = ExplodingViewFactoryTransformer() -private val viewEnvironment = ViewEnvironment(viewRegistry).withCompositionRoot { content -> - Providers(BackgroundColorAmbient provides Color.Green, children = content) -} +private val viewEnvironment = ViewEnvironment(viewRegistry/*.modifyViewFactories(exploder)*/) + .withCompositionRoot { content -> + Providers(BackgroundColorAmbient provides Color.Green, children = content) + } class NestedRenderingsActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - setContentWorkflow(viewEnvironment) { - WorkflowRunner.Config( - RecursiveWorkflow, - diagnosticListener = SimpleLoggingDiagnosticListener() - ) + setContent { + var vibrateRange by state { 0f } + Column { + Slider(value = vibrateRange, onValueChange = { + vibrateRange = it + exploder.min = -it + exploder.max = it + }, valueRange = 0f..10f) + + Providers(BackgroundColorAmbient provides Color.Green) { + WorkflowContainer( + RecursiveWorkflow, + diagnosticListener = remember { SimpleLoggingDiagnosticListener() } + ) { viewEnvironment.showRendering(it) } + } + } } } } diff --git a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveWorkflow.kt b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveWorkflow.kt index c8780445..b35313b8 100644 --- a/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveWorkflow.kt +++ b/samples/nested-renderings/src/main/java/com/squareup/sample/nestedrenderings/RecursiveWorkflow.kt @@ -67,7 +67,12 @@ object RecursiveWorkflow : StatefulWorkflow() { return Rendering( children = List(state.children) { i -> val child = context.renderChild(RecursiveWorkflow, key = i.toString()) - if (i % 2 == 0) child else LegacyRendering(child) + if (i % 2 == 0) child else { + val leafChild = child as Rendering + val l1 = LegacyRendering(leafChild) + val l2 = Rendering(listOf(l1), {},{}) + LegacyRendering(l2) + } }, onAddChildClicked = { context.actionSink.send(addChild()) }, onResetClicked = { context.actionSink.send(reset()) }