Skip to content

Commit 472c71a

Browse files
committed
Sets the intial value of InOverlay before the Dialog is built.
Necessary because we use `OverlayDialogHolder.showing` to get a unique name for the `stateRegistryAggregator.installChildRegistryOwnerOn` call in `DialogSession`. (`OverlayDialogHolder.showing` relies on `InOverlay`.) That call has to happen before the first call to `OverlayDialogHolder.show`, which is what normally sets the value. Chicken and egg type of situation. Fixes #825
1 parent d24b551 commit 472c71a

File tree

3 files changed

+128
-13
lines changed

3 files changed

+128
-13
lines changed

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,13 @@ import android.util.SparseArray
88
import android.widget.EditText
99
import androidx.activity.ComponentActivity
1010
import androidx.test.ext.junit.rules.ActivityScenarioRule
11-
import com.google.common.truth.Truth
1211
import com.google.common.truth.Truth.assertThat
1312
import com.squareup.workflow1.ui.AndroidScreen
1413
import com.squareup.workflow1.ui.Compatible
1514
import com.squareup.workflow1.ui.NamedScreen
1615
import com.squareup.workflow1.ui.Screen
1716
import com.squareup.workflow1.ui.ScreenViewFactory
1817
import com.squareup.workflow1.ui.ScreenViewHolder
19-
import com.squareup.workflow1.ui.ViewEnvironment
2018
import com.squareup.workflow1.ui.ViewEnvironment.Companion.EMPTY
2119
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
2220
import com.squareup.workflow1.ui.show
@@ -32,7 +30,7 @@ internal class BackStackContainerTest {
3230
private data class Rendering(val name: String) : Compatible, AndroidScreen<Rendering> {
3331
override val compatibilityKey = name
3432
override val viewFactory: ScreenViewFactory<Rendering>
35-
get() = ScreenViewFactory.fromCode<Rendering> { _, initialRendering, context, _ ->
33+
get() = ScreenViewFactory.fromCode { _, initialRendering, context, _ ->
3634
ScreenViewHolder(
3735
initialRendering,
3836
EditText(context).apply {
@@ -103,7 +101,7 @@ internal class BackStackContainerTest {
103101

104102
holder.show(BackStackScreen(Rendering("able")), EMPTY)
105103
val showing = view.visibleRendering as Rendering
106-
Truth.assertThat(showing).isEqualTo(Rendering("able"))
104+
assertThat(showing).isEqualTo(Rendering("able"))
107105
}
108106
}
109107

@@ -117,7 +115,7 @@ internal class BackStackContainerTest {
117115
holder.show(BackStackScreen(Rendering("able")), EMPTY)
118116
holder.show(BackStackScreen(Rendering("baker")), EMPTY)
119117
val showing = view.visibleRendering as Rendering
120-
Truth.assertThat(showing).isEqualTo(Rendering("baker"))
118+
assertThat(showing).isEqualTo(Rendering("baker"))
121119
}
122120
}
123121

@@ -132,7 +130,7 @@ internal class BackStackContainerTest {
132130
holder.show(BackStackScreen(Rendering("baker")), EMPTY)
133131
holder.show(BackStackScreen(Rendering("charlie")), EMPTY)
134132
val showing = view.visibleRendering as Rendering
135-
Truth.assertThat(showing).isEqualTo(Rendering("charlie"))
133+
assertThat(showing).isEqualTo(Rendering("charlie"))
136134

137135
// This used to fail because of our naive use of TransitionManager. The
138136
// transition from baker view to charlie view was dropped because the
@@ -152,19 +150,15 @@ internal class BackStackContainerTest {
152150
holder.show(BackStackScreen(Rendering("able")), EMPTY)
153151
holder.show(BackStackScreen(Rendering("able")), EMPTY)
154152

155-
Truth.assertThat(view.transitionCount).isEqualTo(1)
153+
assertThat(view.transitionCount).isEqualTo(1)
156154
}
157155
}
158156

159157
private class VisibleBackStackContainer(context: Context) : BackStackContainer(context) {
160158
var transitionCount = 0
161-
@Suppress("UNCHECKED_CAST") val visibleRendering: Screen?
159+
@Suppress("UNCHECKED_CAST") val visibleRendering: Screen
162160
get() = (getChildAt(0)?.tag as NamedScreen<*>).wrapped
163161

164-
fun show(rendering: BackStackScreen<*>) {
165-
update(rendering, ViewEnvironment.EMPTY)
166-
}
167-
168162
override fun performTransition(
169163
oldHolderMaybe: ScreenViewHolder<NamedScreen<*>>?,
170164
newHolder: ScreenViewHolder<NamedScreen<*>>,
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package com.squareup.workflow1.ui.container
2+
3+
import android.app.Dialog
4+
import android.content.Context
5+
import android.view.View
6+
import android.widget.EditText
7+
import androidx.activity.ComponentActivity
8+
import androidx.test.ext.junit.rules.ActivityScenarioRule
9+
import androidx.test.ext.junit.runners.AndroidJUnit4
10+
import com.google.common.truth.Truth.assertThat
11+
import com.squareup.workflow1.ui.AndroidScreen
12+
import com.squareup.workflow1.ui.Compatible
13+
import com.squareup.workflow1.ui.ScreenViewFactory
14+
import com.squareup.workflow1.ui.ScreenViewHolder
15+
import com.squareup.workflow1.ui.ViewEnvironment
16+
import com.squareup.workflow1.ui.WorkflowLayout
17+
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
18+
import com.squareup.workflow1.ui.container.OverlayDialogHolder.Companion.InOverlay
19+
import com.squareup.workflow1.ui.environmentOrNull
20+
import org.junit.Rule
21+
import org.junit.Test
22+
import org.junit.runner.RunWith
23+
24+
@OptIn(WorkflowUiExperimentalApi::class)
25+
@RunWith(AndroidJUnit4::class)
26+
internal class DialogIntegrationTest {
27+
@get:Rule val scenarioRule = ActivityScenarioRule(ComponentActivity::class.java)
28+
private val scenario get() = scenarioRule.scenario
29+
30+
private data class ContentRendering(val name: String) :
31+
Compatible, AndroidScreen<ContentRendering> {
32+
override val compatibilityKey = name
33+
override val viewFactory: ScreenViewFactory<ContentRendering>
34+
get() = ScreenViewFactory.fromCode { _, initialRendering, context, _ ->
35+
ScreenViewHolder(
36+
initialRendering,
37+
EditText(context).apply {
38+
// Must have an id to participate in view persistence.
39+
id = 65
40+
}
41+
) { _, _ -> /* Noop */ }
42+
}
43+
}
44+
45+
private var latestContentView: View? = null
46+
private var latestDialog: Dialog? = null
47+
48+
private inner class DialogRendering(
49+
name: String,
50+
override val content: ContentRendering
51+
) :
52+
Compatible, AndroidOverlay<DialogRendering>, ScreenOverlay<ContentRendering> {
53+
override val compatibilityKey = name
54+
override val dialogFactory =
55+
object : ScreenOverlayDialogFactory<ContentRendering, DialogRendering>(
56+
type = DialogRendering::class
57+
) {
58+
override fun buildContent(
59+
viewFactory: ScreenViewFactory<ContentRendering>,
60+
initialContent: ContentRendering,
61+
initialEnvironment: ViewEnvironment,
62+
context: Context
63+
): ScreenViewHolder<ContentRendering> =
64+
super.buildContent(viewFactory, initialContent, initialEnvironment, context).also {
65+
latestContentView = it.view
66+
}
67+
68+
override fun buildDialogWithContent(
69+
content: ScreenViewHolder<ContentRendering>
70+
) = super.buildDialogWithContent(content).also { latestDialog = it }
71+
}
72+
}
73+
74+
@Test fun showOne() {
75+
val screen = BodyAndOverlaysScreen(
76+
ContentRendering("body"), DialogRendering("dialog", ContentRendering("content"))
77+
)
78+
79+
scenario.onActivity { activity ->
80+
val root = WorkflowLayout(activity)
81+
root.show(screen)
82+
83+
assertThat(latestContentView).isNotNull()
84+
assertThat(latestDialog).isNotNull()
85+
assertThat(latestDialog!!.isShowing).isTrue()
86+
}
87+
}
88+
89+
/** https://github.com/square/workflow-kotlin/issues/825 */
90+
@Test fun showASecondDialog() {
91+
val oneDialog = BodyAndOverlaysScreen(
92+
ContentRendering("body"), DialogRendering("dialog", ContentRendering("content"))
93+
)
94+
lateinit var root: WorkflowLayout
95+
96+
scenario.onActivity { activity ->
97+
root = WorkflowLayout(activity)
98+
root.show(oneDialog)
99+
}
100+
101+
val dialog2 = DialogRendering("dialog2", ContentRendering("content2"))
102+
val twoDialogs = BodyAndOverlaysScreen(
103+
ContentRendering("body"),
104+
DialogRendering("dialog1", ContentRendering("content1")),
105+
dialog2
106+
)
107+
108+
scenario.onActivity {
109+
root.show(twoDialogs)
110+
val lastOverlay = latestContentView?.environmentOrNull?.get(InOverlay)!!
111+
assertThat(lastOverlay).isEqualTo(dialog2)
112+
}
113+
}
114+
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport
1818
import com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner
1919
import com.squareup.workflow1.ui.androidx.WorkflowSavedStateRegistryAggregator
2020
import com.squareup.workflow1.ui.container.DialogSession.KeyAndBundle
21+
import com.squareup.workflow1.ui.container.OverlayDialogHolder.Companion.InOverlay
2122
import kotlinx.coroutines.flow.MutableStateFlow
2223
import kotlinx.coroutines.flow.StateFlow
2324

@@ -139,7 +140,13 @@ public class LayeredDialogSessions private constructor(
139140

140141
for ((i, overlay) in overlays.withIndex()) {
141142
val covered = i < modalIndex
142-
val dialogEnv = if (covered) envPlusBounds + (CoveredByModal to true) else envPlusBounds
143+
// Seed InOverlay before the Dialog is created, so that it's available to
144+
// DialogSession before the first call to OverlayDialogHolder.show (which is
145+
// what normally sets it).
146+
// https://github.com/square/workflow-kotlin/issues/825
147+
val envPlusInOverlay = envPlusBounds + (InOverlay to overlay)
148+
val dialogEnv =
149+
if (covered) envPlusInOverlay + (CoveredByModal to true) else envPlusInOverlay
143150

144151
updatedSessions += if (i < sessions.size && sessions[i].holder.canShow(overlay)) {
145152
// There is already a dialog at this index, and it is compatible

0 commit comments

Comments
 (0)