Skip to content

Commit b56cf57

Browse files
committed
Brings ViewStarter to the UI update.
Propagates changes from #602 -- replace `initializeView` with `viewStarter` to fix #597.
1 parent a691e8e commit b56cf57

File tree

10 files changed

+167
-84
lines changed

10 files changed

+167
-84
lines changed

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

Lines changed: 98 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import org.junit.Test
1111
internal class DecorativeScreenViewFactoryTest {
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 : ScreenViewFactory<InnerRendering> {
@@ -38,27 +38,25 @@ internal class DecorativeScreenViewFactoryTest {
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(
52-
OuterRendering("outer", InnerRendering("inner")),
50+
OuterRendering("outer", InnerRendering("inner")).buildView(
5351
viewEnvironment,
5452
instrumentation.context
55-
)
53+
).start()
5654

5755
assertThat(events).containsExactly(
58-
"initializeView OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
56+
"viewStarter OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner)) " +
5957
"Updated environment",
6058
"inner showRendering InnerRendering(innerData=inner)",
61-
"exit initializeView"
59+
"exit viewStarter"
6260
)
6361
}
6462

@@ -86,21 +84,97 @@ internal class DecorativeScreenViewFactoryTest {
8684
innerShowRendering(outerRendering.wrapped, env)
8785
}
8886
)
89-
val viewRegistry = ViewRegistry(innerViewFactory)
87+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
9088
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
9189

92-
outerViewFactory.buildView(
93-
OuterRendering("outer", InnerRendering("inner")),
90+
OuterRendering("outer", InnerRendering("inner")).buildView(
9491
viewEnvironment,
9592
instrumentation.context
96-
)
93+
).start()
9794

9895
assertThat(events).containsExactly(
9996
"doShowRendering OuterRendering(outerData=outer, wrapped=InnerRendering(innerData=inner))",
10097
"inner showRendering InnerRendering(innerData=inner)"
10198
)
10299
}
103100

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

@@ -125,14 +199,13 @@ internal class DecorativeScreenViewFactoryTest {
125199
innerShowRendering(outerRendering.wrapped, env)
126200
}
127201
)
128-
val viewRegistry = ViewRegistry(innerViewFactory)
202+
val viewRegistry = ViewRegistry(innerViewFactory, outerViewFactory)
129203
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
130204

131-
val view = outerViewFactory.buildView(
132-
OuterRendering("out1", InnerRendering("in1")),
205+
val view = OuterRendering("out1", InnerRendering("in1")).buildView(
133206
viewEnvironment,
134207
instrumentation.context
135-
)
208+
).apply { start() }
136209
events.clear()
137210

138211
view.showRendering(OuterRendering("out2", InnerRendering("in2")), viewEnvironment)
@@ -149,5 +222,10 @@ internal class DecorativeScreenViewFactoryTest {
149222
val wrapped: InnerRendering
150223
) : Screen
151224

225+
private data class WayOutRendering(
226+
val wayOutData: String,
227+
val wrapped: OuterRendering
228+
) : Screen
229+
152230
private class InnerView(context: Context) : View(context)
153231
}

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/AndroidViewRegistry.kt

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,27 @@ public fun <RenderingT : Any> ViewRegistry.getFactoryFor(
3535
}
3636

3737
@Suppress("DEPRECATION")
38-
@Deprecated("Use ViewEnvironment.buildview")
38+
@Deprecated("Use Screen.buildview")
3939
@WorkflowUiExperimentalApi
4040
public fun <RenderingT : Any> ViewRegistry.buildView(
4141
initialRendering: RenderingT,
4242
initialViewEnvironment: ViewEnvironment,
4343
contextForNewView: Context,
4444
container: ViewGroup? = null,
45-
initializeView: View.() -> Unit = { showFirstRendering() }
45+
viewStarter: ViewStarter? = null,
4646
): View {
4747
return getFactoryForRendering(initialRendering).buildView(
4848
initialRendering, initialViewEnvironment, contextForNewView, container
4949
).also { view ->
50-
checkNotNull(view.showRenderingTag) {
50+
checkNotNull(view.workflowViewStateOrNull) {
5151
"View.bindShowRendering should have been called for $view, typically by the " +
52-
"${ViewFactory::class.java.name} that created it."
52+
"ViewFactory that created it."
53+
}
54+
viewStarter?.let { givenStarter ->
55+
val doStart = view.starter
56+
view.starter = { newView ->
57+
givenStarter.startView(newView) { doStart.invoke(newView) }
58+
}
5359
}
54-
initializeView.invoke(view)
5560
}
5661
}

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/AsScreenViewFactory.kt

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,14 @@ by ManualScreenViewFactory(
1111
initialRendering.rendering,
1212
initialViewEnvironment,
1313
context,
14-
container,
15-
// Don't call showRendering yet, we need to wrap the function first.
16-
initializeView = { }
14+
container
1715
).also { view ->
1816
val legacyShowRendering = view.getShowRendering<Any>()!!
1917

2018
view.bindShowRendering(
2119
initialRendering,
2220
initialViewEnvironment
2321
) { rendering, env -> legacyShowRendering(rendering.rendering, env) }
24-
25-
view.showFirstRendering()
2622
}
2723
}
2824
)

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/DecorativeScreenViewFactory.kt

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ import kotlin.reflect.KClass
6969
* by DecorativeScreenViewFactory(
7070
* type = WithTutorialTips::class,
7171
* map = { withTips -> withTips.wrapped },
72-
* initializeView = {
72+
* viewStarter = { view, doStart ->
7373
* TutorialTipRunner.run(this)
74-
* showFirstRendering<WithTutorialTips<*>>()
74+
* doStart()
7575
* }
7676
* )
7777
*
@@ -109,10 +109,9 @@ import kotlin.reflect.KClass
109109
* @param map called to convert instances of [OuterT] to [InnerT], and to
110110
* allow [ViewEnvironment] to be transformed.
111111
*
112-
* @param initializeView Optional function invoked immediately after the [View] is
113-
* created (that is, immediately after the call to [ScreenViewFactory.buildView]).
114-
* [showRendering], [getRendering] and [environment] are all available when this is called.
115-
* Defaults to a call to [View.showFirstRendering].
112+
* @param viewStarter An optional wrapper for the function invoked when [View.start]
113+
* is called, allowing for last second initialization of a newly built [View].
114+
* See [ViewStarter] for details.
116115
*
117116
* @param doShowRendering called to apply the [ViewShowRendering] function for
118117
* [InnerT], allowing pre- and post-processing. Default implementation simply
@@ -122,7 +121,7 @@ import kotlin.reflect.KClass
122121
public class DecorativeScreenViewFactory<OuterT : Screen, InnerT : Screen>(
123122
override val type: KClass<OuterT>,
124123
private val map: (OuterT, ViewEnvironment) -> Pair<InnerT, ViewEnvironment>,
125-
private val initializeView: View.() -> Unit = { showFirstRendering() },
124+
private val viewStarter: ViewStarter? = null,
126125
private val doShowRendering: (
127126
view: View,
128127
innerShowRendering: ViewShowRendering<InnerT>,
@@ -140,7 +139,7 @@ public class DecorativeScreenViewFactory<OuterT : Screen, InnerT : Screen>(
140139
public constructor(
141140
type: KClass<OuterT>,
142141
map: (OuterT) -> InnerT,
143-
initializeView: View.() -> Unit = { showFirstRendering() },
142+
viewStarter: ViewStarter? = null,
144143
doShowRendering: (
145144
view: View,
146145
innerShowRendering: ViewShowRendering<InnerT>,
@@ -152,7 +151,7 @@ public class DecorativeScreenViewFactory<OuterT : Screen, InnerT : Screen>(
152151
) : this(
153152
type,
154153
map = { outer, viewEnvironment -> Pair(map(outer), viewEnvironment) },
155-
initializeView = initializeView,
154+
viewStarter = viewStarter,
156155
doShowRendering = doShowRendering
157156
)
158157

@@ -168,8 +167,7 @@ public class DecorativeScreenViewFactory<OuterT : Screen, InnerT : Screen>(
168167
processedInitialEnv,
169168
contextForNewView,
170169
container,
171-
// Don't call showRendering yet, we need to wrap the function first.
172-
initializeView = { }
170+
viewStarter
173171
)
174172
.also { view ->
175173
val innerShowRendering: ViewShowRendering<InnerT> = view.getShowRendering()!!
@@ -178,8 +176,6 @@ public class DecorativeScreenViewFactory<OuterT : Screen, InnerT : Screen>(
178176
initialRendering,
179177
processedInitialEnv
180178
) { rendering, env -> doShowRendering(view, innerShowRendering, rendering, env) }
181-
182-
view.initializeView()
183179
}
184180
}
185181
}

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactory.kt

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -42,29 +42,16 @@ public interface ScreenViewFactory<in RenderingT : Screen> : ViewRegistry.Entry<
4242
* It is usually more convenient to use [WorkflowViewStub] or [DecorativeScreenViewFactory]
4343
* than to call this method directly.
4444
*
45-
* Finds a [ScreenViewFactory] to create a [View] to display [this@buildView]. The new view
46-
* can be updated via calls to [View.showRendering] -- that is, it is guaranteed that
47-
* [bindShowRendering] has been called on this view.
45+
* Finds a [ScreenViewFactory] to create a [View] to display the receiving [Screen].
46+
* The caller is responsible for calling [View.start] on the new [View]. After that,
47+
* [View.showRendering] can be used to update it with new renderings that
48+
* are [compatible] with this [Screen]. [WorkflowViewStub] takes care of this chore itself.
4849
*
49-
* The returned view will have a
50-
* [WorkflowLifecycleOwner][com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner]
51-
* set on it. The returned view must EITHER:
50+
* @param viewStarter An optional wrapper for the function invoked when [View.start]
51+
* is called, allowing for last second initialization of a newly built [View].
52+
* See [ViewStarter] for details.
5253
*
53-
* 1. Be attached at least once to ensure that the lifecycle eventually gets destroyed (because its
54-
* parent is destroyed), or
55-
* 2. Have its
56-
* [WorkflowLifecycleOwner.destroyOnDetach][com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner.destroyOnDetach]
57-
* called, which will either schedule the
58-
* lifecycle to be destroyed if the view is attached, or destroy it immediately if it's detached.
59-
*
60-
* [WorkflowViewStub] takes care of this chore itself.
61-
*
62-
* @param initializeView Optional function invoked immediately after the [View] is
63-
* created (that is, immediately after the call to [ScreenViewFactory.buildView]).
64-
* [showRendering], [getRendering] and [environment] are all available when this is called.
65-
* Defaults to a call to [View.showFirstRendering].
66-
*
67-
* @throws IllegalArgumentException if no builder can be find for type [ScreenT]
54+
* @throws IllegalArgumentException if no builder can be found for type [ScreenT]
6855
*
6956
* @throws IllegalStateException if the matching [ScreenViewFactory] fails to call
7057
* [View.bindShowRendering] when constructing the view
@@ -74,27 +61,41 @@ public fun <ScreenT : Screen> ScreenT.buildView(
7461
viewEnvironment: ViewEnvironment,
7562
contextForNewView: Context,
7663
container: ViewGroup? = null,
77-
initializeView: View.() -> Unit = { showFirstRendering() }
64+
viewStarter: ViewStarter? = null,
7865
): View {
7966
val viewFactory = viewEnvironment.getViewFactoryForRendering(this)
8067

8168
return viewFactory.buildView(this, viewEnvironment, contextForNewView, container).also { view ->
82-
checkNotNull(view.showRenderingTag) {
69+
checkNotNull(view.workflowViewStateOrNull) {
8370
"View.bindShowRendering should have been called for $view, typically by the " +
84-
"${ScreenViewFactory::class.java.name} that created it."
71+
"ScreenViewFactory that created it."
72+
}
73+
viewStarter?.let { givenStarter ->
74+
val doStart = view.starter
75+
view.starter = { newView ->
76+
givenStarter.startView(newView) { doStart.invoke(newView) }
77+
}
8578
}
86-
initializeView.invoke(view)
8779
}
8880
}
8981

9082
/**
91-
* Default implementation for the `initializeView` argument of [Screen.buildView],
92-
* and for [DecorativeScreenViewFactory.initializeView]. Calls [showRendering] against
93-
* [getRendering] and [environment].
83+
* A wrapper for the function invoked when [View.start] is called, allowing for
84+
* last second initialization of a newly built [View]. Provided via [Screen.buildView]
85+
* or [DecorativeScreenViewFactory.viewStarter].
86+
*
87+
* While [View.getRendering] may be called from [startView], it is not safe to
88+
* assume that the type of the rendering retrieved matches the type the view was
89+
* originally built to display. [ViewFactories][ViewFactory] can be wrapped, and
90+
* renderings can be mapped to other types.
9491
*/
9592
@WorkflowUiExperimentalApi
96-
public fun View.showFirstRendering() {
97-
showRendering(getRendering()!!, environment!!)
93+
public fun interface ViewStarter {
94+
/** Called from [View.start]. [doStart] must be invoked. */
95+
public fun startView(
96+
view: View,
97+
doStart: () -> Unit
98+
)
9899
}
99100

100101
@WorkflowUiExperimentalApi

0 commit comments

Comments
 (0)