Skip to content

Commit df925e5

Browse files
rjrjrrjrjr
authored
Merge pull request #602 from square/ray/initializeView-is-a-landmine
Replaces *.initializeView with *.viewStarter
2 parents 821c7b6 + b93a522 commit df925e5

File tree

13 files changed

+344
-174
lines changed

13 files changed

+344
-174
lines changed

‎workflow-ui/compose/src/main/java/com/squareup/workflow1/ui/compose/WorkflowRendering.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import com.squareup.workflow1.ui.WorkflowViewStub
2727
import com.squareup.workflow1.ui.getFactoryForRendering
2828
import com.squareup.workflow1.ui.getShowRendering
2929
import com.squareup.workflow1.ui.showRendering
30+
import com.squareup.workflow1.ui.start
3031
import kotlin.reflect.KClass
3132

3233
/**
@@ -184,6 +185,8 @@ private fun <R : Any> ViewFactory<R>.asComposeViewFactory() =
184185
// we don't have access to that.
185186
originalFactory.buildView(rendering, viewEnvironment, context, container = null)
186187
.also { view ->
188+
view.start()
189+
187190
// Mirrors the check done in ViewRegistry.buildView.
188191
checkNotNull(view.getShowRendering<Any>()) {
189192
"View.bindShowRendering should have been called for $view, typically by the " +

‎workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/backstack/BackStackContainer.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ import com.squareup.workflow1.ui.canShowRendering
3232
import com.squareup.workflow1.ui.compatible
3333
import com.squareup.workflow1.ui.container.R
3434
import com.squareup.workflow1.ui.getRendering
35-
import com.squareup.workflow1.ui.showFirstRendering
3635
import com.squareup.workflow1.ui.showRendering
36+
import com.squareup.workflow1.ui.start
3737

3838
/**
3939
* A container view that can display a stream of [BackStackScreen] instances.
@@ -103,11 +103,12 @@ public open class BackStackContainer @JvmOverloads constructor(
103103
initialViewEnvironment = environment,
104104
contextForNewView = this.context,
105105
container = this,
106-
initializeView = {
107-
WorkflowLifecycleOwner.installOn(this)
108-
showFirstRendering()
106+
viewStarter = { view, doStart ->
107+
WorkflowLifecycleOwner.installOn(view)
108+
doStart()
109109
}
110110
)
111+
newView.start()
111112
viewStateCache.update(named.backStack, oldViewMaybe, newView)
112113

113114
val popped = currentRendering?.backStack?.any { compatible(it, named.top) } == true

‎workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.squareup.workflow1.ui.buildView
2020
import com.squareup.workflow1.ui.modal.ModalViewContainer.Companion.binding
2121
import com.squareup.workflow1.ui.onBackPressedDispatcherOwnerOrNull
2222
import com.squareup.workflow1.ui.showRendering
23+
import com.squareup.workflow1.ui.start
2324
import kotlin.reflect.KClass
2425

2526
/**
@@ -72,6 +73,7 @@ public open class ModalViewContainer @JvmOverloads constructor(
7273
container = this
7374
)
7475
.apply {
76+
start()
7577
// If the modal's root view has no backPressedHandler, add a no-op one to
7678
// ensure that the `onBackPressed` call below will not leak up to handlers
7779
// that should be blocked by this modal session.

‎workflow-ui/core-android/api/core-android.api‎

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ public final class com/squareup/workflow1/ui/BuilderViewFactory : com/squareup/w
2424
}
2525

2626
public final class com/squareup/workflow1/ui/DecorativeViewFactory : com/squareup/workflow1/ui/ViewFactory {
27-
public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V
28-
public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
29-
public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;)V
30-
public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
27+
public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;)V
28+
public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
29+
public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;)V
30+
public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function2;Lcom/squareup/workflow1/ui/ViewStarter;Lkotlin/jvm/functions/Function4;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
3131
public fun buildView (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Landroid/view/View;
3232
public fun getType ()Lkotlin/reflect/KClass;
3333
}
@@ -51,21 +51,6 @@ public final class com/squareup/workflow1/ui/LayoutRunnerViewFactory : com/squar
5151
public fun getType ()Lkotlin/reflect/KClass;
5252
}
5353

54-
public final class com/squareup/workflow1/ui/ShowRenderingTag {
55-
public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
56-
public final fun component1 ()Ljava/lang/Object;
57-
public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
58-
public final fun component3 ()Lkotlin/jvm/functions/Function2;
59-
public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/ShowRenderingTag;
60-
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;
61-
public fun equals (Ljava/lang/Object;)Z
62-
public final fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
63-
public final fun getShowRendering ()Lkotlin/jvm/functions/Function2;
64-
public final fun getShowing ()Ljava/lang/Object;
65-
public fun hashCode ()I
66-
public fun toString ()Ljava/lang/String;
67-
}
68-
6954
public final class com/squareup/workflow1/ui/SnapshotParcelsKt {
7055
public static final fun toSnapshot (Landroid/os/Parcelable;)Lcom/squareup/workflow1/Snapshot;
7156
}
@@ -124,21 +109,24 @@ public final class com/squareup/workflow1/ui/ViewRegistry$Companion : com/square
124109
public final class com/squareup/workflow1/ui/ViewRegistryKt {
125110
public static final fun ViewRegistry ()Lcom/squareup/workflow1/ui/ViewRegistry;
126111
public static final fun ViewRegistry ([Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
127-
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;
128-
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;
112+
public static final fun buildView (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;Lcom/squareup/workflow1/ui/ViewStarter;)Landroid/view/View;
113+
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;Lcom/squareup/workflow1/ui/ViewStarter;ILjava/lang/Object;)Landroid/view/View;
129114
public static final fun getFactoryForRendering (Lcom/squareup/workflow1/ui/ViewRegistry;Ljava/lang/Object;)Lcom/squareup/workflow1/ui/ViewFactory;
130115
public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewFactory;)Lcom/squareup/workflow1/ui/ViewRegistry;
131116
public static final fun plus (Lcom/squareup/workflow1/ui/ViewRegistry;Lcom/squareup/workflow1/ui/ViewRegistry;)Lcom/squareup/workflow1/ui/ViewRegistry;
132-
public static final fun showFirstRendering (Landroid/view/View;)V
133117
}
134118

135119
public final class com/squareup/workflow1/ui/ViewShowRenderingKt {
136120
public static final fun bindShowRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
137121
public static final fun canShowRendering (Landroid/view/View;Ljava/lang/Object;)Z
138122
public static final fun getEnvironment (Landroid/view/View;)Lcom/squareup/workflow1/ui/ViewEnvironment;
139-
public static final fun getRendering (Landroid/view/View;)Ljava/lang/Object;
140123
public static final fun getShowRendering (Landroid/view/View;)Lkotlin/jvm/functions/Function2;
141124
public static final fun showRendering (Landroid/view/View;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
125+
public static final fun start (Landroid/view/View;)V
126+
}
127+
128+
public abstract interface class com/squareup/workflow1/ui/ViewStarter {
129+
public abstract fun startView (Landroid/view/View;Lkotlin/jvm/functions/Function0;)V
142130
}
143131

144132
public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout {
@@ -149,6 +137,42 @@ public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/Fra
149137
public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V
150138
}
151139

140+
public abstract class com/squareup/workflow1/ui/WorkflowViewState {
141+
public abstract fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
142+
public abstract fun getShowRendering ()Lkotlin/jvm/functions/Function2;
143+
}
144+
145+
public final class com/squareup/workflow1/ui/WorkflowViewState$New : com/squareup/workflow1/ui/WorkflowViewState {
146+
public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)V
147+
public synthetic fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
148+
public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
149+
public final fun component3 ()Lkotlin/jvm/functions/Function2;
150+
public final fun component4 ()Lkotlin/jvm/functions/Function1;
151+
public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/WorkflowViewState$New;
152+
public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowViewState$New;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowViewState$New;
153+
public fun equals (Ljava/lang/Object;)Z
154+
public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
155+
public fun getShowRendering ()Lkotlin/jvm/functions/Function2;
156+
public synthetic fun getShowing ()Ljava/lang/Object;
157+
public final fun getStarter ()Lkotlin/jvm/functions/Function1;
158+
public fun hashCode ()I
159+
public fun toString ()Ljava/lang/String;
160+
}
161+
162+
public final class com/squareup/workflow1/ui/WorkflowViewState$Started : com/squareup/workflow1/ui/WorkflowViewState {
163+
public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
164+
public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
165+
public final fun component3 ()Lkotlin/jvm/functions/Function2;
166+
public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/WorkflowViewState$Started;
167+
public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/WorkflowViewState$Started;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/WorkflowViewState$Started;
168+
public fun equals (Ljava/lang/Object;)Z
169+
public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
170+
public fun getShowRendering ()Lkotlin/jvm/functions/Function2;
171+
public synthetic fun getShowing ()Ljava/lang/Object;
172+
public fun hashCode ()I
173+
public fun toString ()Ljava/lang/String;
174+
}
175+
152176
public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/View {
153177
public fun <init> (Landroid/content/Context;)V
154178
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V

‎workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/DecorativeViewFactoryTest.kt‎

Lines changed: 99 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import org.junit.Test
1111
internal class DecorativeViewFactoryTest {
1212
private val instrumentation = InstrumentationRegistry.getInstrumentation()
1313

14-
@Test fun initializeView_is_only_call_to_showRendering() {
14+
@Test fun viewStarter_is_only_call_to_showRendering() {
1515
val events = mutableListOf<String>()
1616

1717
val innerViewFactory = object : ViewFactory<InnerRendering> {
@@ -38,27 +38,26 @@ internal class DecorativeViewFactoryTest {
3838
val enhancedEnv = env + (envString to "Updated environment")
3939
Pair(outer.wrapped, enhancedEnv)
4040
},
41-
initializeView = {
42-
val outerRendering = getRendering<OuterRendering>()
43-
events += "initializeView $outerRendering ${environment!![envString]}"
44-
showFirstRendering()
45-
events += "exit initializeView"
41+
viewStarter = { view, doStart ->
42+
events += "viewStarter ${view.getRendering<Any>()} ${view.environment!![envString]}"
43+
doStart()
44+
events += "exit viewStarter"
4645
}
4746
)
48-
val viewRegistry = ViewRegistry(innerViewFactory)
47+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
4948
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
5049

51-
outerViewFactory.buildView(
50+
viewRegistry.buildView(
5251
OuterRendering("outer", InnerRendering("inner")),
5352
viewEnvironment,
5453
instrumentation.context
55-
)
54+
).start()
5655

5756
assertThat(events).containsExactly(
58-
"initializeView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
57+
"viewStarter OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
5958
"Updated environment",
6059
"inner showRendering InnerRendering(innerData=inner)",
61-
"exit initializeView"
60+
"exit viewStarter"
6261
)
6362
}
6463

@@ -86,21 +85,99 @@ internal class DecorativeViewFactoryTest {
8685
innerShowRendering(outerRendering.wrapped, env)
8786
}
8887
)
89-
val viewRegistry = ViewRegistry(innerViewFactory)
88+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
9089
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
9190

92-
outerViewFactory.buildView(
91+
viewRegistry.buildView(
9392
OuterRendering("outer", InnerRendering("inner")),
9493
viewEnvironment,
9594
instrumentation.context
96-
)
95+
).start()
9796

9897
assertThat(events).containsExactly(
9998
"doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))",
10099
"inner showRendering InnerRendering(innerData=inner)"
101100
)
102101
}
103102

103+
// https://github.com/square/workflow-kotlin/issues/597
104+
@Test fun double_wrapping_only_calls_showRendering_once() {
105+
val events = mutableListOf<String>()
106+
107+
val innerViewFactory = object : ViewFactory<InnerRendering> {
108+
override val type = InnerRendering::class
109+
override fun buildView(
110+
initialRendering: InnerRendering,
111+
initialViewEnvironment: ViewEnvironment,
112+
contextForNewView: Context,
113+
container: ViewGroup?
114+
): View = InnerView(contextForNewView).apply {
115+
bindShowRendering(initialRendering, initialViewEnvironment) { rendering, _ ->
116+
events += "inner showRendering $rendering"
117+
}
118+
}
119+
}
120+
121+
val envString = object : ViewEnvironmentKey<String>(String::class) {
122+
override val default: String get() = "Not set"
123+
}
124+
125+
val outerViewFactory = DecorativeViewFactory(
126+
type = OuterRendering::class,
127+
map = { outer, env ->
128+
val enhancedEnv = env + (envString to "Outer Updated environment" +
129+
" SHOULD NOT SEE THIS! It will be clobbered by WayOutRendering")
130+
Pair(outer.wrapped, enhancedEnv)
131+
},
132+
viewStarter = { view, doStart ->
133+
events += "outer viewStarter ${view.getRendering<Any>()} ${view.environment!![envString]}"
134+
doStart()
135+
events += "exit outer viewStarter"
136+
}
137+
)
138+
139+
val wayOutViewFactory = DecorativeViewFactory(
140+
type = WayOutRendering::class,
141+
map = { wayOut, env ->
142+
val enhancedEnv = env + (envString to "Way Out Updated environment triumphs over all")
143+
Pair(wayOut.wrapped, enhancedEnv)
144+
},
145+
viewStarter = { view, doStart ->
146+
events += "way out viewStarter ${view.getRendering<Any>()} ${view.environment!![envString]}"
147+
doStart()
148+
events += "exit way out viewStarter"
149+
}
150+
)
151+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory, wayOutViewFactory)
152+
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
153+
154+
viewRegistry.buildView(
155+
WayOutRendering("way out", OuterRendering("outer", InnerRendering("inner"))),
156+
viewEnvironment,
157+
instrumentation.context
158+
).start()
159+
160+
assertThat(events).containsExactly(
161+
"way out viewStarter " +
162+
"WayOutRendering(wayOutData=way out, wrapped=" +
163+
"OuterRendering(outerData=outer, wrapped=" +
164+
"InnerRendering(innerData=inner))) " +
165+
"Way Out Updated environment triumphs over all",
166+
"outer viewStarter " +
167+
// Notice that both the initial rendering and the ViewEnvironment are stomped by
168+
// the outermost wrapper before inners are invoked. Could try to give
169+
// the inner wrapper access to the rendering it expected, but there are no
170+
// use cases and it trashes the API.
171+
"WayOutRendering(wayOutData=way out, wrapped=" +
172+
"OuterRendering(outerData=outer, wrapped=" +
173+
"InnerRendering(innerData=inner))) " +
174+
"Way Out Updated environment triumphs over all",
175+
"inner showRendering InnerRendering(innerData=inner)",
176+
"exit outer viewStarter",
177+
"exit way out viewStarter"
178+
)
179+
}
180+
104181
@Test fun subsequent_showRendering_calls_wrapped_showRendering() {
105182
val events = mutableListOf<String>()
106183

@@ -125,14 +202,14 @@ internal class DecorativeViewFactoryTest {
125202
innerShowRendering(outerRendering.wrapped, env)
126203
}
127204
)
128-
val viewRegistry = ViewRegistry(innerViewFactory)
205+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
129206
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
130207

131-
val view = outerViewFactory.buildView(
208+
val view = viewRegistry.buildView(
132209
OuterRendering("out1", InnerRendering("in1")),
133210
viewEnvironment,
134211
instrumentation.context
135-
)
212+
).apply { start() }
136213
events.clear()
137214

138215
view.showRendering(OuterRendering("out2", InnerRendering("in2")), viewEnvironment)
@@ -149,5 +226,10 @@ internal class DecorativeViewFactoryTest {
149226
val wrapped: InnerRendering
150227
)
151228

229+
private data class WayOutRendering(
230+
val wayOutData: String,
231+
val wrapped: OuterRendering
232+
)
233+
152234
private class InnerView(context: Context) : View(context)
153235
}

0 commit comments

Comments
 (0)