From d37e6cd69b108afe37f34789aaa1123e455da078 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Wed, 8 Dec 2021 17:15:49 -0800 Subject: [PATCH] Replaces *.initializeView with *.viewStarter Fixes #597. It used to be the case that every `DecorativeViewFactory` called `showRendering()` twice (#397). We fixed that (we thought) by introducing the `initializeView` lambda to `ViewRegistry.buildView` and `DecorativeViewFactory` (#408). Unfortunately, that fix botched recursion. Individual `DecorativeViewFactory` instances work fine, but if you wrap them you still get one `showRendering` call from each. Worse, upstream `initializeView` lambdas are clobbered by immediately downstream ones. e.g., when a `WorkflowViewStub` shows a `DecorativeViewFactory`, the `WorkflowLifecycleRunner.installOn` call in the former is clobbered. The fix is to completely decouple building a view from from this kind of initialization. `ViewRegistry.buildView` and its wrappers no longer try to call `showRendering` at all. Instead the caller of `buildView` (mostly `WorkflowViewStub`) is reponsible for immediately calling `View.start` on the new `View`. `View.start` makes the initial `showRendering` call that formerly was the job of `ViewFactory.buildView` -- the factory builds the view, and the container turns the key. Since `View.start` is called only after all wrapped `ViewFactory.buildView` functions have executed, we're certain it will only happen once. Of course we still need the ability to customize view initialization via wrapping, especially to invoke `WorkflowLifecycleOwner.installOn`. To accomodate that, the function that `View.start` executes can be wrapped via the new `viewStarter` argument to `ViewRegistry.buildView` and `DecorativeViewFactory`, which replaces `initializeView`. This required a pretty thorough overhaul of `ViewShowRendering.kt`, which is renamed `WorkflowUiViewState.kt`. `WorkflowUiViewState` is a sealed class that hangs off of a view tag, with two implementations (`New` and `Started`) to help us enforce the order of the `ViewRegistry.buildView`, `View.bindShowRendering`, `View.start` and `View.showRendering` calls. --- .../workflow1/ui/compose/WorkflowRendering.kt | 3 + .../ui/backstack/BackStackContainer.kt | 9 +- workflow-ui/core-android/api/core-android.api | 82 +++++--- .../workflow1/ui/DecorativeViewFactoryTest.kt | 78 +++---- .../workflow1/ui/DecorativeViewFactory.kt | 22 +- .../com/squareup/workflow1/ui/ViewRegistry.kt | 42 ++-- .../workflow1/ui/ViewShowRendering.kt | 136 ------------ .../workflow1/ui/WorkflowUiViewState.kt | 197 ++++++++++++++++++ .../squareup/workflow1/ui/WorkflowViewStub.kt | 8 +- .../core-android/src/main/res/values/ids.xml | 2 +- .../squareup/workflow1/ui/TestViewFactory.kt | 4 +- 11 files changed, 337 insertions(+), 246 deletions(-) delete mode 100644 workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt create mode 100644 workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowUiViewState.kt diff --git a/workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt b/workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt index 7b086a53a7..22e33f07ee 100644 --- a/workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt +++ b/workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt @@ -27,6 +27,7 @@ import com.squareup.workflow1.ui.WorkflowViewStub import com.squareup.workflow1.ui.getFactoryForRendering import com.squareup.workflow1.ui.getShowRendering import com.squareup.workflow1.ui.showRendering +import com.squareup.workflow1.ui.start import kotlin.reflect.KClass /** @@ -184,6 +185,8 @@ private fun ViewFactory.asComposeViewFactory() = // we don't have access to that. originalFactory.buildView(rendering, viewEnvironment, context, container = null) .also { view -> + view.start() + // Mirrors the check done in ViewRegistry.buildView. checkNotNull(view.getShowRendering()) { "View.bindShowRendering should have been called for $view, typically by the " + diff --git a/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt b/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt index 72bcd19d32..3fdf611e7c 100644 --- a/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt +++ b/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt @@ -32,8 +32,8 @@ import com.squareup.workflow1.ui.canShowRendering import com.squareup.workflow1.ui.compatible import com.squareup.workflow1.ui.container.R import com.squareup.workflow1.ui.getRendering -import com.squareup.workflow1.ui.showFirstRendering import com.squareup.workflow1.ui.showRendering +import com.squareup.workflow1.ui.start /** * A container view that can display a stream of [BackStackScreen] instances. @@ -103,11 +103,12 @@ public open class BackStackContainer @JvmOverloads constructor( initialViewEnvironment = environment, contextForNewView = this.context, container = this, - initializeView = { - WorkflowLifecycleOwner.installOn(this) - showFirstRendering() + viewStarter = { view, doStart -> + WorkflowLifecycleOwner.installOn(view) + doStart() } ) + newView.start() viewStateCache.update(named.backStack, oldViewMaybe, newView) val popped = currentRendering?.backStack?.any { compatible(it, named.top) } == true diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 4cc208b366..4a45e0ed8d 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -24,10 +24,10 @@ public final class com/squareup/workflow1/ui/BuilderViewFactory : com/squareup/w } public final class com/squareup/workflow1/ui/DecorativeViewFactory : com/squareup/workflow1/ui/ViewFactory { - public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V - public synthetic fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V - public synthetic fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/OnStartWrapper;Lkotlin/jvm/functions/Function4;)V + public synthetic fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/OnStartWrapper;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/OnStartWrapper;Lkotlin/jvm/functions/Function4;)V + public synthetic fun (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/OnStartWrapper;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun buildView (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View; public fun getType ()Lkotlin/reflect/KClass; } @@ -51,19 +51,8 @@ public final class com/squareup/workflow1/ui/LayoutRunnerViewFactory : com/squar public fun getType ()Lkotlin/reflect/KClass; } -public final class com/squareup/workflow1/ui/ShowRenderingTag { - public fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)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 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 getShowing ()Ljava/lang/Object; - public fun hashCode ()I - public fun toString ()Ljava/lang/String; +public abstract interface class com/squareup/workflow1/ui/OnStartWrapper { + public abstract fun startView (Landroid/view/View;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)V } public final class com/squareup/workflow1/ui/SnapshotParcelsKt { @@ -124,21 +113,11 @@ 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;Lkotlin/jvm/functions/Function1;)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/Function1;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;)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 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 static final fun showFirstRendering (Landroid/view/View;)V -} - -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 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 showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V } public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout { @@ -149,6 +128,51 @@ public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/Fra public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V } +public abstract class com/squareup/workflow1/ui/WorkflowUiViewState { + public abstract fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment; + public abstract fun getShowRendering ()Lkotlin/jvm/functions/Function2; +} + +public final class com/squareup/workflow1/ui/WorkflowUiViewState$New : com/squareup/workflow1/ui/WorkflowUiViewState { + public fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)V + public synthetic fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment; + public final fun component3 ()Lkotlin/jvm/functions/Function2; + public final fun component4 ()Lkotlin/jvm/functions/Function1; + public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$New; + public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowUiViewState$New;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$New; + public fun equals (Ljava/lang/Object;)Z + public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment; + public fun getShowRendering ()Lkotlin/jvm/functions/Function2; + public synthetic fun getShowing ()Ljava/lang/Object; + public final fun getStarter ()Lkotlin/jvm/functions/Function1; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public final class com/squareup/workflow1/ui/WorkflowUiViewState$Started : com/squareup/workflow1/ui/WorkflowUiViewState { + public fun (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V + 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/WorkflowUiViewState$Started; + public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowUiViewState$Started;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowUiViewState$Started; + public fun equals (Ljava/lang/Object;)Z + public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment; + public fun getShowRendering ()Lkotlin/jvm/functions/Function2; + public synthetic fun getShowing ()Ljava/lang/Object; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public final class com/squareup/workflow1/ui/WorkflowUiViewStateKt { + 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 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 getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function2; + public static final fun showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V + public static final fun start (Landroid/view/View;)V +} + public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/View { public fun (Landroid/content/Context;)V public fun (Landroid/content/Context;Landroid/util/AttributeSet;)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 9999e4d579..90fe3193c8 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 @@ -11,7 +11,7 @@ import org.junit.Test internal class DecorativeViewFactoryTest { private val instrumentation = InstrumentationRegistry.getInstrumentation() - @Test fun initializeView_is_only_call_to_showRendering() { + @Test fun onStartWrapper_is_only_call_to_showRendering() { val events = mutableListOf() val innerViewFactory = object : ViewFactory { @@ -38,27 +38,26 @@ internal class DecorativeViewFactoryTest { val enhancedEnv = env + (envString to "Updated environment") Pair(outer.wrapped, enhancedEnv) }, - initializeView = { - val outerRendering = getRendering() - events += "initializeView $outerRendering ${environment!![envString]}" - showFirstRendering() - events += "exit initializeView" + viewStarter = { view, doStart -> + events += "onStartWrapper ${view.getRendering()} ${view.environment!![envString]}" + doStart() + events += "exit onStartWrapper" } ) - val viewRegistry = ViewRegistry(innerViewFactory) + val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory) val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry)) - outerViewFactory.buildView( + viewRegistry.buildView( OuterRendering("outer", InnerRendering("inner")), viewEnvironment, instrumentation.context - ) + ).start() assertThat(events).containsExactly( - "initializeView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " + + "onStartWrapper OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " + "Updated environment", "inner showRendering InnerRendering(innerData=inner)", - "exit initializeView" + "exit onStartWrapper" ) } @@ -86,14 +85,14 @@ internal class DecorativeViewFactoryTest { innerShowRendering(outerRendering.wrapped, env) } ) - val viewRegistry = ViewRegistry(innerViewFactory) + val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory) val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry)) - outerViewFactory.buildView( + viewRegistry.buildView( OuterRendering("outer", InnerRendering("inner")), viewEnvironment, instrumentation.context - ) + ).start() assertThat(events).containsExactly( "doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))", @@ -126,28 +125,27 @@ internal class DecorativeViewFactoryTest { val outerViewFactory = DecorativeViewFactory( type = OuterRendering::class, map = { outer, env -> - val enhancedEnv = env + (envString to "Outer Updated environment") + val enhancedEnv = env + (envString to "Outer Updated environment" + + " SHOULD NOT SEE THIS! It will be clobbered by WayOutRendering") Pair(outer.wrapped, enhancedEnv) }, - initializeView = { - val outerRendering = getRendering() - events += "outer initializeView $outerRendering ${environment!![envString]}" - showFirstRendering() - events += "exit outer initializeView" + viewStarter = { view, doStart -> + events += "outer onStartWrapper ${view.getRendering()} ${view.environment!![envString]}" + doStart() + events += "exit outer onStartWrapper" } ) val wayOutViewFactory = DecorativeViewFactory( type = WayOutRendering::class, map = { wayOut, env -> - val enhancedEnv = env + (envString to "Way Out Updated environment") + val enhancedEnv = env + (envString to "Way Out Updated environment triumphs over all") Pair(wayOut.wrapped, enhancedEnv) }, - initializeView = { - val wayOutRendering = getRendering() - events += "way out initializeView $wayOutRendering ${environment!![envString]}" - showFirstRendering() - events += "exit way out initializeView" + viewStarter = { view, doStart -> + events += "way out onStartWrapper ${view.getRendering()} ${view.environment!![envString]}" + doStart() + events += "exit way out onStartWrapper" } ) val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory, wayOutViewFactory) @@ -157,21 +155,26 @@ internal class DecorativeViewFactoryTest { WayOutRendering("way out", OuterRendering("outer", InnerRendering("inner"))), viewEnvironment, instrumentation.context - ) + ).start() assertThat(events).containsExactly( - "way out initializeView " + + "way out onStartWrapper " + "WayOutRendering(wayOutData=way out, wrapped=" + "OuterRendering(outerData=outer, wrapped=" + "InnerRendering(innerData=inner))) " + - "Way Out Updated environment", - "outer initializeView " + + "Way Out Updated environment triumphs over all", + "outer onStartWrapper " + + // Notice that both the initial rendering and the ViewEnvironment are stomped by + // the outermost wrapper before inners are invoked. Could try to give + // the inner wrapper access to the rendering it expected, but there are no + // use cases and it trashes the API. + "WayOutRendering(wayOutData=way out, wrapped=" + "OuterRendering(outerData=outer, wrapped=" + - "InnerRendering(innerData=inner)) " + - "Outer Updated environment", + "InnerRendering(innerData=inner))) " + + "Way Out Updated environment triumphs over all", "inner showRendering InnerRendering(innerData=inner)", - "exit outer initializeView", - "exit way out initializeView" + "exit outer onStartWrapper", + "exit way out onStartWrapper" ) } @@ -199,14 +202,14 @@ internal class DecorativeViewFactoryTest { innerShowRendering(outerRendering.wrapped, env) } ) - val viewRegistry = ViewRegistry(innerViewFactory) + val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory) val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry)) - val view = outerViewFactory.buildView( + val view = viewRegistry.buildView( OuterRendering("out1", InnerRendering("in1")), viewEnvironment, instrumentation.context - ) + ).apply { start() } events.clear() view.showRendering(OuterRendering("out2", InnerRendering("in2")), viewEnvironment) @@ -222,6 +225,7 @@ internal class DecorativeViewFactoryTest { val outerData: String, val wrapped: InnerRendering ) + private data class WayOutRendering( val wayOutData: String, val wrapped: OuterRendering 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 4f24ca3253..b7748bd5f6 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 @@ -61,9 +61,9 @@ import kotlin.reflect.KClass * by DecorativeViewFactory( * type = WithTutorialTips::class, * map = { withTips -> withTips.wrapped }, - * initializeView = { - * TutorialTipRunner.run(this) - * showFirstRendering>() + * onStartWrapper = { view, withTutorialTips, starter -> + * TutorialTipRunner.run(view) + * starter(view) * } * ) * @@ -100,10 +100,7 @@ import kotlin.reflect.KClass * @param map called to convert instances of [OuterT] to [InnerT], and to * allow [ViewEnvironment] to be transformed. * - * @param initializeView Optional function invoked immediately after the [View] is - * created (that is, immediately after the call to [ViewFactory.buildView]). - * [showRendering], [getRendering] and [environment] are all available when this is called. - * Defaults to a call to [View.showFirstRendering]. + * @param viewStarter DOCUMENT ME RAY * * @param doShowRendering called to apply the [ViewShowRendering] function for * [InnerT], allowing pre- and post-processing. Default implementation simply @@ -113,7 +110,7 @@ import kotlin.reflect.KClass public class DecorativeViewFactory( override val type: KClass, private val map: (OuterT, ViewEnvironment) -> Pair, - private val initializeView: View.() -> Unit = { showFirstRendering() }, + private val viewStarter: ViewStarter? = null, private val doShowRendering: ( view: View, innerShowRendering: ViewShowRendering, @@ -131,7 +128,7 @@ public class DecorativeViewFactory( public constructor( type: KClass, map: (OuterT) -> InnerT, - initializeView: View.() -> Unit = { showFirstRendering() }, + viewStarter: ViewStarter? = null, doShowRendering: ( view: View, innerShowRendering: ViewShowRendering, @@ -143,7 +140,7 @@ public class DecorativeViewFactory( ) : this( type, map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) }, - initializeView = initializeView, + viewStarter = viewStarter, doShowRendering = doShowRendering ) @@ -161,8 +158,7 @@ public class DecorativeViewFactory( processedInitialEnv, contextForNewView, container, - // Don't call showRendering yet, we need to wrap the function first. - initializeView = { } + viewStarter ) .also { view -> val innerShowRendering: ViewShowRendering = view.getShowRendering()!! @@ -171,8 +167,6 @@ public class DecorativeViewFactory( initialRendering, processedInitialEnv ) { rendering, env -> doShowRendering(view, innerShowRendering, rendering, env) } - - view.initializeView() } } } 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 6fd84cf762..59e4237393 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 @@ -143,7 +143,11 @@ public fun * 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 + * Finds a [ViewFactory] to create a [View] ready to display [initialRendering]. The caller + * is responsible for calling [View.start]. After that, [View.showRendering] can be used + * to update the new [View] with new renderings that are [compatible] with [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. * @@ -158,12 +162,7 @@ public fun * called, which will either schedule the * lifecycle to be destroyed if the view is attached, or destroy it immediately if it's detached. * - * @param initializeView Optional function invoked immediately after the [View] is - * created (that is, immediately after the call to [ViewFactory.buildView]). - * [showRendering], [getRendering] and [environment] are all available when this is called. - * Defaults to a call to [View.showFirstRendering]. - * - * @throws IllegalArgumentException if no factory can be find for type [RenderingT] + * @throws IllegalArgumentException if no factory can be found for type [RenderingT] * * @throws IllegalStateException if the matching [ViewFactory] fails to call * [View.bindShowRendering] when constructing the view @@ -174,19 +173,32 @@ public fun ViewRegistry.buildView( initialViewEnvironment: ViewEnvironment, contextForNewView: Context, container: ViewGroup? = null, - initializeView: View.() -> Unit = { showFirstRendering() } + viewStarter: ViewStarter? = null, ): View { return getFactoryForRendering(initialRendering).buildView( initialRendering, initialViewEnvironment, contextForNewView, container ).also { view -> - checkNotNull(view.showRenderingTag) { + checkNotNull(view.workflowTagOrNull) { "View.bindShowRendering should have been called for $view, typically by the " + "${ViewFactory::class.java.name} that created it." } - initializeView.invoke(view) + viewStarter?.let { givenStarter -> + val doStart = view.starter + view.starter = { newView -> + givenStarter.startView(newView) { doStart.invoke(newView) } + } + } } } +@WorkflowUiExperimentalApi +public fun interface ViewStarter { + public fun startView( + view: View, + doStart: () -> Unit + ) +} + @WorkflowUiExperimentalApi public operator fun ViewRegistry.plus(binding: ViewFactory<*>): ViewRegistry = this + ViewRegistry(binding) @@ -194,13 +206,3 @@ public operator fun ViewRegistry.plus(binding: ViewFactory<*>): ViewRegistry = @WorkflowUiExperimentalApi public operator fun ViewRegistry.plus(other: ViewRegistry): ViewRegistry = CompositeViewRegistry(this, other) - -/** - * Default implementation for the `initializeView` argument of [ViewRegistry.buildView], - * and for [DecorativeViewFactory.initializeView]. Calls [showRendering] against - * [getRendering] and [environment]. - */ -@WorkflowUiExperimentalApi -public fun View.showFirstRendering() { - showRendering(getRendering()!!, environment!!) -} 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 deleted file mode 100644 index bb84966898..0000000000 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ViewShowRendering.kt +++ /dev/null @@ -1,136 +0,0 @@ -package com.squareup.workflow1.ui - -import android.view.View - -/** - * Function attached to a view created by [ViewFactory], to allow it - * to respond to [View.showRendering]. - */ -@WorkflowUiExperimentalApi -public typealias ViewShowRendering = - (@UnsafeVariance RenderingT, ViewEnvironment) -> Unit - -/** -` * View tag that holds the function to make the view show instances of [RenderingT], and - * the [current rendering][showing]. - * - * @param showing the current rendering. Used by [canShowRendering] to decide if the - * view can be updated with the next rendering. - */ -@WorkflowUiExperimentalApi -public data class ShowRenderingTag( - val showing: RenderingT, - val environment: ViewEnvironment, - val showRendering: ViewShowRendering -) - -/** - * Establishes [showRendering] as the implementation of [View.showRendering] - * 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( - initialRendering: RenderingT, - initialViewEnvironment: ViewEnvironment, - showRendering: ViewShowRendering -) { - setTag( - R.id.view_show_rendering_function, - ShowRenderingTag(initialRendering, initialViewEnvironment, showRendering) - ) -} - -/** - * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. - * - * True if this view is able to show [rendering]. - * - * Returns `false` if [bindShowRendering] has not been called, so it is always safe to - * call this method. Otherwise returns the [compatibility][compatible] of the initial - * [rendering] and the new one. - */ -@WorkflowUiExperimentalApi -public fun View.canShowRendering(rendering: Any): Boolean { - return getRendering()?.matches(rendering) == true -} - -/** - * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. - * - * Sets the workflow rendering associated with this view, and displays it by - * invoking the [ViewShowRendering] function previously set by [bindShowRendering]. - * - * @throws IllegalStateException if [bindShowRendering] has not been called. - */ -@WorkflowUiExperimentalApi -public fun View.showRendering( - rendering: RenderingT, - viewEnvironment: ViewEnvironment -) { - showRenderingTag - ?.let { tag -> - check(tag.showing.matches(rendering)) { - "Expected $this to be able to show rendering $rendering, but that did not match " + - "previous rendering ${tag.showing}. " + - "Consider using ${WorkflowViewStub::class.java.simpleName} to display arbitrary types." - } - - // Update the tag's rendering and viewEnvironment. - bindShowRendering(rendering, viewEnvironment, tag.showRendering) - // And do the actual showRendering work. - tag.showRendering.invoke(rendering, viewEnvironment) - } - ?: error( - "Expected $this to have a showRendering function to show $rendering. " + - "Perhaps it was not built by a ${ViewFactory::class.java.simpleName}, " + - "or perhaps the factory did not call View.bindShowRendering." - ) -} - -/** - * Returns the most recent rendering shown by this view, or null if [bindShowRendering] - * has never been called. - */ -@WorkflowUiExperimentalApi -public fun View.getRendering(): RenderingT? { - // Can't use a val because of the parameter type. - @Suppress("UNCHECKED_CAST") - return when (val showing = showRenderingTag?.showing) { - null -> null - else -> showing as RenderingT - } -} - -/** - * Returns the most recent [ViewEnvironment] that apply to this view, or null if [bindShowRendering] - * has never been called. - */ -@WorkflowUiExperimentalApi -public val View.environment: ViewEnvironment? - get() = showRenderingTag?.environment - -/** - * Returns the function set by the most recent call to [bindShowRendering], or null - * if that method has never been called. - */ -@WorkflowUiExperimentalApi -public fun View.getShowRendering(): ViewShowRendering? { - return showRenderingTag?.showRendering -} - -/** - * Returns the [ShowRenderingTag] established by the last call to [View.bindShowRendering], - * or null if that method has never been called. - */ -@WorkflowUiExperimentalApi -@PublishedApi -internal val View.showRenderingTag: ShowRenderingTag<*>? - get() = getTag(R.id.view_show_rendering_function) as? ShowRenderingTag<*> - -@WorkflowUiExperimentalApi -private fun Any.matches(other: Any) = compatible(this, other) diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowUiViewState.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowUiViewState.kt new file mode 100644 index 0000000000..448e54e5dc --- /dev/null +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowUiViewState.kt @@ -0,0 +1,197 @@ +package com.squareup.workflow1.ui + +import android.view.View +import com.squareup.workflow1.ui.WorkflowUiViewState.New +import com.squareup.workflow1.ui.WorkflowUiViewState.Started + +/** + * Function attached to a view created by [ViewFactory], to allow it + * to respond to [View.showRendering]. + */ +@WorkflowUiExperimentalApi +public typealias ViewShowRendering = + (@UnsafeVariance RenderingT, ViewEnvironment) -> Unit + +/** + * [View tag][View.setTag] that holds the functions and state backing [View.showRendering], etc. + */ +@WorkflowUiExperimentalApi +@PublishedApi +internal sealed class WorkflowUiViewState { + @PublishedApi + internal abstract val showing: RenderingT + abstract val environment: ViewEnvironment + abstract val showRendering: ViewShowRendering + + /** [bindShowRendering] has been called, [start] has not. */ + data class New( + override val showing: RenderingT, + override val environment: ViewEnvironment, + override val showRendering: ViewShowRendering, + + val starter: (View) -> Unit = { view -> + view.showRendering(view.getRendering()!!, view.environment!!) + } + ) : WorkflowUiViewState() + + /** [start] has been called. It's safe to call [showRendering] now. */ + data class Started( + override val showing: RenderingT, + override val environment: ViewEnvironment, + override val showRendering: ViewShowRendering + ) : WorkflowUiViewState() +} + +/** + * Intended for use by implementations of [ViewFactory.buildView]. + * + * Establishes [showRendering] as the implementation of [View.showRendering] + * for the receiver, possibly replacing the existing one. Likewise sets / updates + * the values returned by [View.getRendering] and [View.environment]. + * + * - [View.start] must be called exactly once before [View.showRendering]. + * + * @throws IllegalStateException when called after [View.start] + * + * @see DecorativeViewFactory + */ +@WorkflowUiExperimentalApi +public fun View.bindShowRendering( + initialRendering: RenderingT, + initialViewEnvironment: ViewEnvironment, + showRendering: ViewShowRendering +) { + workflowTag = when (workflowTagOrNull) { + is New<*> -> New(initialRendering, initialViewEnvironment, showRendering, starter) + else -> New(initialRendering, initialViewEnvironment, showRendering) + } +} + +/** + * Function called by [View.start]. Exposed to allow callers of [ViewRegistry.buildView] + * to customize that method by wrapping this function. It is an error to use this property + * after calling [View.start]. + */ +@WorkflowUiExperimentalApi +internal var View.starter: (View) -> Unit + get() = workflowTagAsNew.starter + set(value) { + workflowTag = workflowTagAsNew.copy(starter = value) + } + +/** + * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. + * + * Makes the initial call to [View.showRendering], along with any wrappers that have been + * added via [ViewRegistry.buildView], or [DecorativeViewFactory.viewStarter]. + * + * - It is an error to call this method more than once. + * - It is an error to call [View.showRendering] without having called this method first. + */ +@WorkflowUiExperimentalApi +public fun View.start() { + val current = workflowTagAsNew + workflowTag = Started(current.showing, current.environment, current.showRendering) + current.starter(this) +} + +/** + * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. + * + * True if this view is able to show [rendering]. + * + * Returns `false` if [View.bindShowRendering] has not been called, so it is always safe to + * call this method. Otherwise returns the [compatibility][compatible] of the current + * [View.getRendering] and the new one. + */ +@WorkflowUiExperimentalApi +public fun View.canShowRendering(rendering: Any): Boolean { + return getRendering()?.matches(rendering) == true +} + +/** + * It is usually more convenient to use [WorkflowViewStub] than to call this method directly. + * + * Sets the rendering associated with this view, and displays it by invoking + * the [ViewShowRendering] function previously set by [bindShowRendering]. + * + * @throws IllegalStateException if [bindShowRendering] has not been called. + */ +@WorkflowUiExperimentalApi +public fun View.showRendering( + rendering: RenderingT, + viewEnvironment: ViewEnvironment +) { + workflowTagAsStarted.let { tag -> + check(tag.showing.matches(rendering)) { + "Expected $this to be able to show rendering $rendering, but that did not match " + + "previous rendering ${tag.showing}. " + + "Consider using ${WorkflowViewStub::class.java.simpleName} to display arbitrary types." + } + + // Update the tag's rendering and viewEnvironment. + workflowTag = Started(rendering, viewEnvironment, tag.showRendering) + // And do the actual showRendering work. + tag.showRendering.invoke(rendering, viewEnvironment) + } +} + +/** + * Returns the most recent rendering shown by this view cast to [RenderingT], + * or null if [bindShowRendering] has never been called. + * + * @throws ClassCastException if the current rendering is not of type [RenderingT] + */ +@WorkflowUiExperimentalApi +public inline fun View.getRendering(): RenderingT? { + // Can't use a val because of the parameter type. + return when (val showing = workflowTagOrNull?.showing) { + null -> null + else -> showing as RenderingT + } +} + +/** + * Returns the most recent [ViewEnvironment] applied to this view, or null if [bindShowRendering] + * has never been called. + */ +@WorkflowUiExperimentalApi +public val View.environment: ViewEnvironment? + get() = workflowTagOrNull?.environment + +/** + * Returns the function set by the most recent call to [bindShowRendering], or null + * if that method has never been called. + */ +@WorkflowUiExperimentalApi +public fun View.getShowRendering(): ViewShowRendering? { + return workflowTagOrNull?.showRendering +} + +@WorkflowUiExperimentalApi +@PublishedApi +internal val View.workflowTagOrNull: WorkflowUiViewState<*>? + get() = getTag(R.id.workflow_ui_view_state) as? WorkflowUiViewState<*> + +@WorkflowUiExperimentalApi +private var View.workflowTag: WorkflowUiViewState<*> + get() = workflowTagOrNull ?: error( + "Expected $this to have been built by a ViewFactory. " + + "Perhaps the factory did not call View.bindShowRendering." + ) + set(value) = setTag(R.id.workflow_ui_view_state, value) + +@WorkflowUiExperimentalApi +private val View.workflowTagAsNew: New<*> + get() = workflowTag as? New<*> ?: error( + "Expected $this to be un-started, but View.start() has been called" + ) + +@WorkflowUiExperimentalApi +private val View.workflowTagAsStarted: Started<*> + get() = workflowTag as? Started<*> ?: error( + "Expected $this to have been started, but View.start() has not been called" + ) + +@WorkflowUiExperimentalApi +private fun Any.matches(other: Any) = compatible(this, other) 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 a290e2149b..b5056acbb5 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 @@ -225,12 +225,14 @@ public class WorkflowViewStub @JvmOverloads constructor( viewEnvironment, parent.context, parent, - initializeView = { - WorkflowLifecycleOwner.installOn(this) - showFirstRendering() + viewStarter = { view, doStart -> + WorkflowLifecycleOwner.installOn(view) + doStart() } ) .also { newView -> + newView.start() + if (inflatedId != NO_ID) newView.id = inflatedId if (updatesVisibility) newView.visibility = visibility background?.let { newView.background = it } diff --git a/workflow-ui/core-android/src/main/res/values/ids.xml b/workflow-ui/core-android/src/main/res/values/ids.xml index 1e13c033cf..e0f89acb61 100644 --- a/workflow-ui/core-android/src/main/res/values/ids.xml +++ b/workflow-ui/core-android/src/main/res/values/ids.xml @@ -1,7 +1,7 @@ - + diff --git a/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/TestViewFactory.kt b/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/TestViewFactory.kt index a718275c8c..21b83f8f25 100644 --- a/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/TestViewFactory.kt +++ b/workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/TestViewFactory.kt @@ -25,8 +25,8 @@ internal class TestViewFactory(override val type: KClass) : ViewFact called = true return mock { on { - getTag(eq(com.squareup.workflow1.ui.R.id.view_show_rendering_function)) - } doReturn (ShowRenderingTag(initialRendering, initialViewEnvironment, { _, _ -> })) + getTag(eq(com.squareup.workflow1.ui.R.id.workflow_ui_view_state)) + } doReturn (WorkflowUiViewState(initialRendering, initialViewEnvironment, { _, _ -> })) } } }