From a301ef5143ea2eb36704e1908f0bec5146fa0ef1 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Thu, 15 Apr 2021 17:03:35 -0700 Subject: [PATCH] bindShowRendering no longer calls showRendering. That was always a weird coupling, and it caused the nasty compounding double update behavior in `DecorativeViewFactory`. To prevent `ViewFactory` implementations from having to remember to call `showRendering`, we make `ViewRegistry.buildView` do so. And to allow more sophisticiated `ViewFactory` implementations to prevent that call from happening prematurely, we build it into the default value for a new `initializeView` method on `ViewRegistry.buildView`. `DecorativeViewFactory` takes advantage by passing a no-op function in for `initializeView` when the view is built, and then making its own explicit call to `showRendering` after it's done playing wrapping games. Another interesting change is that the `showRendering` function now gets the `View` as its receiver. Made it that much easier for `ViewRegistry.buildView` to invoke it, and I suspect it will come in handy in other places too. Closes #397 --- .../sample/container/BackButtonViewFactory.kt | 2 +- .../sample/container/panel/PanelContainer.kt | 14 ++-- .../sample/container/panel/ScrimContainer.kt | 34 ++++---- .../backstack/test/BackStackContainerTest.kt | 2 +- .../test/fixtures/BackStackTestActivity.kt | 7 +- .../NoTransitionBackStackContainer.kt | 6 +- .../ui/backstack/BackStackContainer.kt | 11 ++- workflow-ui/core-android/api/core-android.api | 20 ++--- .../workflow1/ui/DecorativeViewFactoryTest.kt | 33 ++++---- .../workflow1/ui/DecorativeViewFactory.kt | 51 +++++++----- .../com/squareup/workflow1/ui/LayoutRunner.kt | 11 +-- .../workflow1/ui/LayoutRunnerViewFactory.kt | 13 ++- .../workflow1/ui/ViewBindingViewFactory.kt | 16 ++-- .../com/squareup/workflow1/ui/ViewRegistry.kt | 79 ++++++++++--------- .../workflow1/ui/ViewShowRendering.kt | 13 +-- .../squareup/workflow1/ui/WorkflowViewStub.kt | 40 +++++----- .../workflow1/ui/modal/AlertContainer.kt | 4 +- .../workflow1/ui/modal/ModalViewContainer.kt | 11 ++- 18 files changed, 200 insertions(+), 167 deletions(-) diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt index 214b977529..8807bc107f 100644 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt +++ b/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt @@ -21,7 +21,7 @@ by DecorativeViewFactory( view.backPressedHandler = outerRendering.onBackPressed } - innerShowRendering.invoke(outerRendering.wrapped, viewEnvironment) + innerShowRendering.invoke(view, outerRendering.wrapped, viewEnvironment) if (outerRendering.override) { // Place our handler after invoking innerShowRendering, so that ours wins. diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/panel/PanelContainer.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/panel/PanelContainer.kt index ff617117b9..10f9290b6b 100644 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/panel/PanelContainer.kt +++ b/samples/containers/android/src/main/java/com/squareup/sample/container/panel/PanelContainer.kt @@ -59,13 +59,15 @@ class PanelContainer @JvmOverloads constructor( } companion object : ViewFactory> by BuilderViewFactory( - type = PanelContainerScreen::class, - viewConstructor = { initialRendering, initialEnv, contextForNewView, _ -> - PanelContainer(contextForNewView).apply { - id = R.id.panel_container - layoutParams = ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT) - bindShowRendering(initialRendering, initialEnv, ::update) + type = PanelContainerScreen::class, + viewConstructor = { initialRendering, initialEnv, contextForNewView, _ -> + PanelContainer(contextForNewView).apply { + id = R.id.panel_container + layoutParams = ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT) + bindShowRendering(initialRendering, initialEnv) { rendering, env -> + update(rendering, env) } } + } ) } diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/panel/ScrimContainer.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/panel/ScrimContainer.kt index 36be903fcb..0c07f53291 100644 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/panel/ScrimContainer.kt +++ b/samples/containers/android/src/main/java/com/squareup/sample/container/panel/ScrimContainer.kt @@ -7,8 +7,8 @@ import android.view.View import android.view.ViewGroup import com.squareup.sample.container.R import com.squareup.workflow1.ui.BuilderViewFactory -import com.squareup.workflow1.ui.WorkflowUiExperimentalApi import com.squareup.workflow1.ui.ViewFactory +import com.squareup.workflow1.ui.WorkflowUiExperimentalApi import com.squareup.workflow1.ui.WorkflowViewStub import com.squareup.workflow1.ui.bindShowRendering @@ -33,7 +33,7 @@ class ScrimContainer @JvmOverloads constructor( private val child: View get() = getChildAt(0) - ?: error("Child must be set immediately upon creation.") + ?: error("Child must be set immediately upon creation.") var isDimmed: Boolean = false set(value) { @@ -84,7 +84,7 @@ class ScrimContainer @JvmOverloads constructor( ValueAnimator.ofFloat(1f, 0f) }.apply { duration = resources.getInteger(android.R.integer.config_shortAnimTime) - .toLong() + .toLong() addUpdateListener { animation -> scrim.alpha = animation.animatedValue as Float } start() } @@ -92,22 +92,20 @@ class ScrimContainer @JvmOverloads constructor( @OptIn(WorkflowUiExperimentalApi::class) companion object : ViewFactory> by BuilderViewFactory( - type = ScrimContainerScreen::class, - viewConstructor = { initialRendering, initialViewEnvironment, contextForNewView, _ -> - val stub = WorkflowViewStub(contextForNewView) + type = ScrimContainerScreen::class, + viewConstructor = { initialRendering, initialViewEnvironment, contextForNewView, _ -> + val stub = WorkflowViewStub(contextForNewView) - ScrimContainer(contextForNewView) - .apply { - layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) - addView(stub) + ScrimContainer(contextForNewView) + .apply { + layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) + addView(stub) - bindShowRendering( - initialRendering, initialViewEnvironment - ) { rendering, environment -> - stub.update(rendering.wrapped, environment) - isDimmed = rendering.dimmed - } - } - } + bindShowRendering(initialRendering, initialViewEnvironment) { rendering, environment -> + stub.update(rendering.wrapped, environment) + isDimmed = rendering.dimmed + } + } + } ) } diff --git a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackStackContainerTest.kt b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackStackContainerTest.kt index 9f56f2061d..c0b8755bb6 100644 --- a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackStackContainerTest.kt +++ b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackStackContainerTest.kt @@ -21,7 +21,7 @@ import org.junit.runner.RunWith @OptIn(WorkflowUiExperimentalApi::class) @RunWith(AndroidJUnit4::class) -class BackStackContainerTest { +internal class BackStackContainerTest { @Rule @JvmField val scenarioRule = ActivityScenarioRule(BackStackTestActivity::class.java) private val scenario get() = scenarioRule.scenario diff --git a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/BackStackTestActivity.kt b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/BackStackTestActivity.kt index 7968b88048..de75aa7c80 100644 --- a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/BackStackTestActivity.kt +++ b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/BackStackTestActivity.kt @@ -25,7 +25,7 @@ import com.squareup.workflow1.ui.showRendering * [onRetainNonConfigurationInstance]. */ @OptIn(WorkflowUiExperimentalApi::class) -class BackStackTestActivity : Activity() { +internal class BackStackTestActivity : Activity() { /** * A simple string holder that creates [ViewStateTestView]s with their ID and tag derived from @@ -35,7 +35,7 @@ class BackStackTestActivity : Activity() { * @param onViewCreated An optional function that will be called by the view factory after the * view is created but before [bindShowRendering]. */ - class TestRendering( + internal class TestRendering( val name: String, val onViewCreated: (ViewStateTestView) -> Unit = {}, val onShowRendering: (ViewStateTestView) -> Unit = {}, @@ -58,7 +58,7 @@ class BackStackTestActivity : Activity() { viewHooks = initialRendering.viewHooks initialRendering.onViewCreated(this) bindShowRendering(initialRendering, initialViewEnvironment) { rendering, _ -> - rendering.onShowRendering(this) + rendering.onShowRendering(this as ViewStateTestView) viewHooks = rendering.viewHooks } } @@ -96,6 +96,7 @@ class BackStackTestActivity : Activity() { check(backstackContainer == null) backstackContainer = NoTransitionBackStackContainer.buildView(backstack!!, viewEnvironment, this) + backstackContainer!!.showRendering(backstack!!, viewEnvironment) setContentView(backstackContainer) } diff --git a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/NoTransitionBackStackContainer.kt b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/NoTransitionBackStackContainer.kt index a740ee3c04..acc097f901 100644 --- a/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/NoTransitionBackStackContainer.kt +++ b/workflow-ui/backstack-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/fixtures/NoTransitionBackStackContainer.kt @@ -16,7 +16,7 @@ import com.squareup.workflow1.ui.bindShowRendering * actual backstack logic. Views are just swapped instantly. */ @OptIn(WorkflowUiExperimentalApi::class) -class NoTransitionBackStackContainer(context: Context) : BackStackContainer(context) { +internal class NoTransitionBackStackContainer(context: Context) : BackStackContainer(context) { override fun performTransition(oldViewMaybe: View?, newView: View, popped: Boolean) { oldViewMaybe?.let(::removeView) @@ -31,7 +31,9 @@ class NoTransitionBackStackContainer(context: Context) : BackStackContainer(cont .apply { id = R.id.workflow_back_stack_container layoutParams = LayoutParams(MATCH_PARENT, MATCH_PARENT) - bindShowRendering(initialRendering, initialEnv, ::update) + bindShowRendering(initialRendering, initialEnv) { rendering, env -> + update(rendering, env) + } } } ) diff --git a/workflow-ui/backstack-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt b/workflow-ui/backstack-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt index 9a9841f5be..5be176b67b 100644 --- a/workflow-ui/backstack-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt +++ b/workflow-ui/backstack-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt @@ -66,7 +66,12 @@ public open class BackStackContainer @JvmOverloads constructor( return } - val newView = environment[ViewRegistry].buildView(named.top, environment, this) + val newView = environment[ViewRegistry].buildView( + initialRendering = named.top, + initialViewEnvironment = environment, + contextForNewView = this.context, + container = this + ) viewStateCache.update(named.backStack, oldViewMaybe, newView) val popped = currentRendering?.backStack?.any { compatible(it, named.top) } == true @@ -146,7 +151,9 @@ public open class BackStackContainer @JvmOverloads constructor( .apply { id = R.id.workflow_back_stack_container layoutParams = (ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) - bindShowRendering(initialRendering, initialEnv, ::update) + bindShowRendering(initialRendering, initialEnv) { rendering, env -> + update(rendering, env) + } } } ) diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index c8ce7f5a25..bf5646b766 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -59,15 +59,15 @@ public final class com/squareup/workflow1/ui/NamedViewFactory : com/squareup/wor } public final class com/squareup/workflow1/ui/ShowRenderingTag { - public fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V + public fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function3;)V public final fun component1 ()Ljava/lang/Object; public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment; - public final fun component3 ()Lkotlin/jvm/functions/Function2; - public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/ShowRenderingTag; - public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/ShowRenderingTag;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/ShowRenderingTag; + public final fun component3 ()Lkotlin/jvm/functions/Function3; + public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function3;)Lcom/squareup/workflow1/ui/ShowRenderingTag; + public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/ShowRenderingTag;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function3;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/ShowRenderingTag; public fun equals (Ljava/lang/Object;)Z public final fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment; - public final fun getShowRendering ()Lkotlin/jvm/functions/Function2; + public final fun getShowRendering ()Lkotlin/jvm/functions/Function3; public final fun getShowing ()Ljava/lang/Object; public fun hashCode ()I public fun toString ()Ljava/lang/String; @@ -127,19 +127,19 @@ public final class com/squareup/workflow1/ui/ViewRegistry$Companion : com/square public final class com/squareup/workflow1/ui/ViewRegistryKt { public static final fun ViewRegistry ()Lcom/squareup/workflow1/ui/ViewRegistry; public static final fun ViewRegistry ([Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry; - public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View; - public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/view/ViewGroup;)Landroid/view/View; - public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;ILjava/lang/Object;)Landroid/view/View; + public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function3;)Landroid/view/View; + public static synthetic fun buildView$default (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lkotlin/jvm/functions/Function3;ILjava/lang/Object;)Landroid/view/View; + public static final fun getFactoryForRendering (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;)Lcom/squareup/workflow1/ui/ViewFactory; public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry; public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewRegistry;)Lcom/squareup/workflow1/ui/ViewRegistry; } public final class com/squareup/workflow1/ui/ViewShowRenderingKt { - public static final fun bindShowRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V + public static final fun bindShowRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function3;)V public static final fun canShowRendering (Landroid/view/View;Ljava/lang/Object;)Z public static final fun getEnvironment (Landroid/view/View;)Lcom/squareup/workflow1/ui/ViewEnvironment; public static final fun getRendering (Landroid/view/View;)Ljava/lang/Object; - public static final fun getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function2; + public static final fun getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function3; public static final fun showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V } diff --git a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt index d6af83051c..764cd83352 100644 --- a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt +++ b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt @@ -8,7 +8,7 @@ import com.google.common.truth.Truth.assertThat import org.junit.Test @OptIn(WorkflowUiExperimentalApi::class) -class DecorativeViewFactoryTest { +internal class DecorativeViewFactoryTest { private val instrumentation = InstrumentationRegistry.getInstrumentation() @@ -28,11 +28,19 @@ class DecorativeViewFactoryTest { } } } + + val envString = object : ViewEnvironmentKey(String::class) { + override val default: String get() = "Not set" + } + val outerViewFactory = DecorativeViewFactory( type = OuterRendering::class, - map = { outer -> outer.wrapped }, - initView = { outerRendering, _ -> - events += "initView $outerRendering" + map = { outer, env -> + val enhancedEnv = env + (envString to "Updated environment") + Pair(outer.wrapped, enhancedEnv) + }, + initView = { outerRendering, view -> + events += "initView $outerRendering ${view.environment!![envString]}" } ) val viewRegistry = ViewRegistry(innerViewFactory) @@ -44,11 +52,9 @@ class DecorativeViewFactoryTest { instrumentation.context ) - // Note that showRendering is called twice. Technically this behavior is not incorrect, although - // it's not necessary. Fix coming soon. assertThat(events).containsExactly( - "inner showRendering InnerRendering(innerData=inner)", - "initView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))", + "initView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " + + "Updated environment", "inner showRendering InnerRendering(innerData=inner)" ) } @@ -72,9 +78,9 @@ class DecorativeViewFactoryTest { val outerViewFactory = DecorativeViewFactory( type = OuterRendering::class, map = { outer -> outer.wrapped }, - doShowRendering = { _, innerShowRendering, outerRendering, env -> + doShowRendering = { view, innerShowRendering, outerRendering, env -> events += "doShowRendering $outerRendering" - innerShowRendering(outerRendering.wrapped, env) + view.innerShowRendering(outerRendering.wrapped, env) } ) val viewRegistry = ViewRegistry(innerViewFactory) @@ -86,10 +92,7 @@ class DecorativeViewFactoryTest { instrumentation.context ) - // Note that showRendering is called twice. Technically this behavior is not incorrect, although - // it's not necessary. Fix coming soon. assertThat(events).containsExactly( - "inner showRendering InnerRendering(innerData=inner)", "doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))", "inner showRendering InnerRendering(innerData=inner)" ) @@ -114,9 +117,9 @@ class DecorativeViewFactoryTest { val outerViewFactory = DecorativeViewFactory( type = OuterRendering::class, map = { outer -> outer.wrapped }, - doShowRendering = { _, innerShowRendering, outerRendering, env -> + doShowRendering = { view, innerShowRendering, outerRendering, env -> events += "doShowRendering $outerRendering" - innerShowRendering(outerRendering.wrapped, env) + view.innerShowRendering(outerRendering.wrapped, env) } ) val viewRegistry = ViewRegistry(innerViewFactory) diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeViewFactory.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeViewFactory.kt index ac701139c0..e1e643e32e 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeViewFactory.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeViewFactory.kt @@ -116,9 +116,9 @@ public class DecorativeViewFactory( innerShowRendering: ViewShowRendering, outerRendering: OuterT, env: ViewEnvironment - ) -> Unit = { _, innerShowRendering, outerRendering, viewEnvironment -> + ) -> Unit = { view, innerShowRendering, outerRendering, viewEnvironment -> val (innerRendering, processedEnv) = map(outerRendering, viewEnvironment) - innerShowRendering(innerRendering, processedEnv) + view.innerShowRendering(innerRendering, processedEnv) } ) : ViewFactory { /** @@ -133,14 +133,14 @@ public class DecorativeViewFactory( innerShowRendering: ViewShowRendering, outerRendering: OuterT, env: ViewEnvironment - ) -> Unit = { _, innerShowRendering, outerRendering, viewEnvironment -> - innerShowRendering(map(outerRendering), viewEnvironment) + ) -> Unit = { view, innerShowRendering, outerRendering, viewEnvironment -> + view.innerShowRendering(map(outerRendering), viewEnvironment) } ) : this( - type, - map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) }, - initView = initView, - doShowRendering = doShowRendering + type, + map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) }, + initView = initView, + doShowRendering = doShowRendering ) override fun buildView( @@ -152,18 +152,27 @@ public class DecorativeViewFactory( val (innerInitialRendering, processedInitialEnv) = map(initialRendering, initialViewEnvironment) return processedInitialEnv[ViewRegistry] - .buildView( - innerInitialRendering, - processedInitialEnv, - contextForNewView, - container - ) - .also { view -> - val innerShowRendering: ViewShowRendering = view.getShowRendering()!! - initView(initialRendering, view) - view.bindShowRendering(initialRendering, processedInitialEnv) { rendering, env -> - doShowRendering(view, innerShowRendering, rendering, env) - } - } + .buildView( + innerInitialRendering, + processedInitialEnv, + contextForNewView, + container, + // Don't call showRendering yet, we need to wrap the function first. + initializeView = { _, _ -> } + ) + .also { view -> + val innerShowRendering: ViewShowRendering = view.getShowRendering()!! + view.bindShowRendering( + initialRendering, + processedInitialEnv + ) { rendering, env -> doShowRendering(view, innerShowRendering, rendering, env) } + + // Call this after we fuss with the bindings, to ensure it can pull the updated + // ViewEnvironment. + initView(initialRendering, view) + + // Our showRendering wrapper is in place, now call it. + view.showRendering(initialRendering, processedInitialEnv) + } } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunner.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunner.kt index de62132f4e..a39f1b0281 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunner.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunner.kt @@ -6,7 +6,6 @@ import android.view.View import android.view.ViewGroup import androidx.annotation.LayoutRes import androidx.viewbinding.ViewBinding -import com.squareup.workflow1.ui.LayoutRunner.Companion.bind @WorkflowUiExperimentalApi public typealias ViewBindingInflater = (LayoutInflater, ViewGroup?, Boolean) -> BindingT @@ -93,16 +92,10 @@ public fun interface LayoutRunner { * Creates a [ViewFactory] that inflates [layoutId] to "show" renderings of type [RenderingT], * with a no-op [LayoutRunner]. Handy for showing static views, e.g. when prototyping. */ + @Suppress("unused") public inline fun bindNoRunner( @LayoutRes layoutId: Int - ): ViewFactory = bind(layoutId) { - object : LayoutRunner { - override fun showRendering( - rendering: RenderingT, - viewEnvironment: ViewEnvironment - ) = Unit - } - } + ): ViewFactory = bind(layoutId) { LayoutRunner { _, _ -> } } } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunnerViewFactory.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunnerViewFactory.kt index 7e5c3810ed..2a25ad5fc6 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunnerViewFactory.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/LayoutRunnerViewFactory.kt @@ -25,13 +25,12 @@ internal class LayoutRunnerViewFactory( container: ViewGroup? ): View { return contextForNewView.viewBindingLayoutInflater(container) - .inflate(layoutId, container, false) - .apply { - bindShowRendering( - initialRendering, - initialViewEnvironment, - runnerConstructor.invoke(this)::showRendering - ) + .inflate(layoutId, container, false) + .also { view -> + val runner = runnerConstructor(view) + view.bindShowRendering(initialRendering, initialViewEnvironment) { rendering, environment -> + runner.showRendering(rendering, environment) } + } } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewBindingViewFactory.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewBindingViewFactory.kt index 4465383636..4b1f215e13 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewBindingViewFactory.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewBindingViewFactory.kt @@ -20,12 +20,14 @@ internal class ViewBindingViewFactory( container: ViewGroup? ): View = bindingInflater(contextForNewView.viewBindingLayoutInflater(container), container, false) - .also { binding -> - binding.root.bindShowRendering( - initialRendering, - initialViewEnvironment, - runnerConstructor.invoke(binding)::showRendering - ) + .also { binding -> + val runner = runnerConstructor(binding) + binding.root.bindShowRendering( + initialRendering, + initialViewEnvironment + ) { rendering, environment -> + runner.showRendering(rendering, environment) } - .root + } + .root } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewRegistry.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewRegistry.kt index b584b609a4..377fc6606d 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewRegistry.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewRegistry.kt @@ -98,55 +98,43 @@ public fun ViewRegistry(vararg bindings: ViewFactory<*>): ViewRegistry = public fun ViewRegistry(): ViewRegistry = TypedViewRegistry() /** - * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. + * It is usually more convenient to use [WorkflowViewStub] or [DecorativeViewFactory] + * than to call this method directly. * - * Creates a [View] to display [initialRendering], which can be updated via calls - * to [View.showRendering]. Prefers entries found via [ViewRegistry.getFactoryFor]. - * If that returns null, falls back to the factory provided by the rendering's - * implementation of [AndroidViewRendering.viewFactory], if there is one. + * Returns the [ViewFactory] that builds [View] instances suitable to display the given [rendering], + * via subsequent calls to [View.showRendering]. * - * @throws IllegalArgumentException if no factory can be find for type [RenderingT] - * - * @throws IllegalStateException if the matching [ViewFactory] fails to call - * [View.bindShowRendering] when constructing the view + * Prefers factories found via [ViewRegistry.getFactoryFor]. If that returns null, falls + * back to the factory provided by the rendering's implementation of + * [AndroidViewRendering.viewFactory], if there is one. Note that this means that a + * compile time [AndroidViewRendering.viewFactory] binding can be overridden at runtime. */ @WorkflowUiExperimentalApi -public fun ViewRegistry.buildView( - initialRendering: RenderingT, - initialViewEnvironment: ViewEnvironment, - contextForNewView: Context, - container: ViewGroup? = null -): View { +public fun + ViewRegistry.getFactoryForRendering(rendering: RenderingT): ViewFactory { @Suppress("UNCHECKED_CAST") - val factory: ViewFactory = getFactoryFor(initialRendering::class) - ?: (initialRendering as? AndroidViewRendering<*>)?.viewFactory as? ViewFactory + return getFactoryFor(rendering::class) + ?: (rendering as? AndroidViewRendering<*>)?.viewFactory as? ViewFactory ?: throw IllegalArgumentException( "A ${ViewFactory::class.qualifiedName} should have been registered to display " + - "${initialRendering::class.qualifiedName} instances, or that class should implement " + - "${AndroidViewRendering::class.simpleName}<${initialRendering::class.simpleName}>." + "${rendering::class.qualifiedName} instances, or that class should implement " + + "${AndroidViewRendering::class.simpleName}<${rendering::class.simpleName}>." ) - - return factory.buildView( - initialRendering, - initialViewEnvironment, - contextForNewView, - container - ) - .apply { - check(this.getRendering() != null) { - "View.bindShowRendering should have been called for $this, typically by the " + - "${ViewFactory::class.java.name} that created it." - } - } } /** - * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. + * It is usually more convenient to use [WorkflowViewStub] or [DecorativeViewFactory] + * than to call this method directly. + * + * Finds a [ViewFactory] to create a [View] to display [initialRendering]. The new view + * can be updated via calls to [View.showRendering] -- that is, it is guaranteed that + * [bindShowRendering] has been called on this view. * - * Creates a [View] to display [initialRendering], which can be updated via calls - * to [View.showRendering]. + * @param initializeView Optional function invoked immediately after the [View] is + * created (that is, immediately after the call to [ViewFactory.buildView]). Defaults + * to a call to [View.showRendering]. * - * @throws IllegalArgumentException if no binding can be find for type [RenderingT] + * @throws IllegalArgumentException if no factory can be find for type [RenderingT] * * @throws IllegalStateException if the matching [ViewFactory] fails to call * [View.bindShowRendering] when constructing the view @@ -155,8 +143,23 @@ public fun ViewRegistry.buildView( public fun ViewRegistry.buildView( initialRendering: RenderingT, initialViewEnvironment: ViewEnvironment, - container: ViewGroup -): View = buildView(initialRendering, initialViewEnvironment, container.context, container) + contextForNewView: Context, + container: ViewGroup? = null, + initializeView: ViewShowRendering = { rendering, env -> + showRendering(rendering, env) + } +): View { + return getFactoryForRendering(initialRendering).buildView( + initialRendering, initialViewEnvironment, contextForNewView, container + ).also { view -> + val tag = checkNotNull(view.showRenderingTag) { + "View.bindShowRendering should have been called for $view, typically by the " + + "${ViewFactory::class.java.name} that created it." + } + @Suppress("UNCHECKED_CAST") + initializeView.invoke(view, tag.showing as RenderingT, tag.environment) + } +} @WorkflowUiExperimentalApi public operator fun ViewRegistry.plus(binding: ViewFactory<*>): ViewRegistry = diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt index d781c91246..6bcfc99b69 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt @@ -8,7 +8,7 @@ import android.view.View */ @WorkflowUiExperimentalApi public typealias ViewShowRendering = - (@UnsafeVariance RenderingT, ViewEnvironment) -> Unit + View.(@UnsafeVariance RenderingT, ViewEnvironment) -> Unit /** ` * View tag that holds the function to make the view show instances of [RenderingT], and @@ -25,13 +25,13 @@ public data class ShowRenderingTag( ) /** - * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. - * * Establishes [showRendering] as the implementation of [View.showRendering] - * for the receiver, possibly replacing the existing one. Immediately invokes [showRendering] - * to display [initialRendering]. + * for the receiver, possibly replacing the existing one. Likewise sets / updates + * the values returned by [View.getRendering] and [View.environment]. * * Intended for use by implementations of [ViewFactory.buildView]. + * + * @see DecorativeViewFactory */ @WorkflowUiExperimentalApi public fun View.bindShowRendering( @@ -43,7 +43,6 @@ public fun View.bindShowRendering( R.id.view_show_rendering_function, ShowRenderingTag(initialRendering, initialViewEnvironment, showRendering) ) - showRendering.invoke(initialRendering, initialViewEnvironment) } /** @@ -81,7 +80,9 @@ public fun View.showRendering( "Consider using ${WorkflowViewStub::class.java.simpleName} to display arbitrary types." } + // Update the tag's rendering and viewEnvironment. bindShowRendering(rendering, viewEnvironment, tag.showRendering) + tag.showRendering.invoke(this, rendering, viewEnvironment) } ?: error( "Expected $this to have a showRendering function to show $rendering. " + diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt index ab6d994852..1c43b7a67a 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt @@ -93,7 +93,7 @@ public class WorkflowViewStub @JvmOverloads constructor( init { val attrs = context.obtainStyledAttributes( - attributeSet, R.styleable.WorkflowViewStub, defStyle, defStyleRes + attributeSet, R.styleable.WorkflowViewStub, defStyle, defStyleRes ) inflatedId = attrs.getResourceId(R.styleable.WorkflowViewStub_inflatedId, NO_ID) updatesVisibility = attrs.getBoolean(R.styleable.WorkflowViewStub_updatesVisibility, true) @@ -121,8 +121,8 @@ public class WorkflowViewStub @JvmOverloads constructor( val index = parent.indexOfChild(actual) parent.removeView(actual) actual.layoutParams - ?.let { parent.addView(newView, index, it) } - ?: run { parent.addView(newView, index) } + ?.let { parent.addView(newView, index, it) } + ?: run { parent.addView(newView, index) } } /** @@ -195,23 +195,27 @@ public class WorkflowViewStub @JvmOverloads constructor( viewEnvironment: ViewEnvironment ): View { actual.takeIf { it.canShowRendering(rendering) } - ?.let { - it.showRendering(rendering, viewEnvironment) - return it - } + ?.let { + it.showRendering(rendering, viewEnvironment) + return it + } val parent = actual.parent as? ViewGroup - ?: throw IllegalStateException( - "WorkflowViewStub must have a non-null ViewGroup parent" - ) + ?: throw IllegalStateException("WorkflowViewStub must have a non-null ViewGroup parent") - return viewEnvironment[ViewRegistry].buildView(rendering, viewEnvironment, parent) - .also { newView -> - if (inflatedId != NO_ID) newView.id = inflatedId - if (updatesVisibility) newView.visibility = visibility - background?.let { newView.background = it } - replaceOldViewInParent(parent, newView) - actual = newView - } + return viewEnvironment[ViewRegistry] + .buildView( + rendering, + viewEnvironment, + parent.context, + parent + ) + .also { newView -> + if (inflatedId != NO_ID) newView.id = inflatedId + if (updatesVisibility) newView.visibility = visibility + background?.let { newView.background = it } + replaceOldViewInParent(parent, newView) + actual = newView + } } } diff --git a/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/AlertContainer.kt b/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/AlertContainer.kt index d768392b36..f4c201f8fe 100644 --- a/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/AlertContainer.kt +++ b/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/AlertContainer.kt @@ -86,7 +86,9 @@ public class AlertContainer @JvmOverloads constructor( .apply { id = R.id.workflow_alert_container layoutParams = ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT) - bindShowRendering(initialRendering, initialEnv, ::update) + bindShowRendering(initialRendering, initialEnv) { rendering, env -> + update(rendering, env) + } } } ) diff --git a/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt b/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt index deeab75fa5..d47c1b775d 100644 --- a/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt +++ b/workflow-ui/modal-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt @@ -62,7 +62,12 @@ public open class ModalViewContainer @JvmOverloads constructor( initialViewEnvironment: ViewEnvironment ): DialogRef { val view = initialViewEnvironment[ViewRegistry] - .buildView(initialModalRendering, initialViewEnvironment, this) + .buildView( + initialRendering = initialModalRendering, + initialViewEnvironment = initialViewEnvironment, + contextForNewView = this.context, + container = this + ) .apply { // If the modal's root view has no backPressedHandler, add a no-op one to // ensure that the `onBackPressed` call below will not leak up to handlers @@ -113,7 +118,9 @@ public open class ModalViewContainer @JvmOverloads constructor( ModalViewContainer(context).apply { this.id = id layoutParams = ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT) - bindShowRendering(initialRendering, initialEnv, ::update) + bindShowRendering(initialRendering, initialEnv) { rendering, env -> + update(rendering, env) + } } } )