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 ) {