Skip to content

Commit a7c1b38

Browse files
rjrjrrjrjr
authored
Merge pull request #1220 from square/ray/name-backstackscreen
Introduces `BackStackScreen.name`
2 parents d4cc5b4 + 678ee20 commit a7c1b38

File tree

6 files changed

+115
-53
lines changed

6 files changed

+115
-53
lines changed

‎samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import com.squareup.sample.container.R
77
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Detail
88
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Overview
99
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Single
10-
import com.squareup.workflow1.ui.NamedScreen
1110
import com.squareup.workflow1.ui.ScreenViewFactory
1211
import com.squareup.workflow1.ui.ScreenViewRunner
1312
import com.squareup.workflow1.ui.ViewEnvironment
@@ -64,7 +63,7 @@ class OverviewDetailContainer(view: View) : ScreenViewRunner<OverviewDetailScree
6463

6564
// Without this name, the two BackStackScreen containers will try
6665
// to sign up with SavedStateRegistry with the same id, and crash.
67-
val overviewRendering = NamedScreen(rendering.overviewRendering, "Overview")
66+
val overviewRendering = rendering.overviewRendering.withName("Overview")
6867
overviewStub!!.show(overviewRendering, overviewViewEnvironment)
6968

7069
rendering.detailRendering

‎workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowSavedStateRegistryAggregator.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ public class WorkflowSavedStateRegistryAggregator {
162162
"Error registering SavedStateProvider: key \"$key\" is already in " +
163163
"use on parent SavedStateRegistryOwner $parentOwner. " +
164164
"This is most easily remedied by giving your container Screen rendering a unique " +
165-
"Compatible.compatibilityKey, perhaps by wrapping it with Named.",
165+
"Compatible.compatibilityKey -- note the name fields on BodyAndOverlaysScreen " +
166+
"and BackStackScreen.",
166167
e
167168
)
168169
}

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,31 +209,39 @@ public final class com/squareup/workflow1/ui/navigation/BackStackConfigKt {
209209
public static final fun plus (Lcom/squareup/workflow1/ui/ViewEnvironment;Lcom/squareup/workflow1/ui/navigation/BackStackConfig;)Lcom/squareup/workflow1/ui/ViewEnvironment;
210210
}
211211

212-
public final class com/squareup/workflow1/ui/navigation/BackStackScreen : com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen {
212+
public final class com/squareup/workflow1/ui/navigation/BackStackScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen {
213213
public static final field Companion Lcom/squareup/workflow1/ui/navigation/BackStackScreen$Companion;
214214
public fun <init> (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V
215+
public fun <init> (Ljava/lang/String;Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V
215216
public fun asSequence ()Lkotlin/sequences/Sequence;
216217
public fun equals (Ljava/lang/Object;)Z
217218
public final fun get (I)Lcom/squareup/workflow1/ui/Screen;
218219
public final fun getBackStack ()Ljava/util/List;
220+
public fun getCompatibilityKey ()Ljava/lang/String;
219221
public final fun getFrames ()Ljava/util/List;
222+
public final fun getName ()Ljava/lang/String;
220223
public final fun getTop ()Lcom/squareup/workflow1/ui/Screen;
221224
public fun hashCode ()I
222225
public synthetic fun map (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/Container;
223226
public fun map (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
224227
public final fun mapIndexed (Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
225228
public fun toString ()Ljava/lang/String;
229+
public final fun withName (Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
226230
}
227231

228232
public final class com/squareup/workflow1/ui/navigation/BackStackScreen$Companion {
229-
public final fun fromList (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
230-
public final fun fromListOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
233+
public final fun fromList (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
234+
public static synthetic fun fromList$default (Lcom/squareup/workflow1/ui/navigation/BackStackScreen$Companion;Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
235+
public final fun fromListOrNull (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
236+
public static synthetic fun fromListOrNull$default (Lcom/squareup/workflow1/ui/navigation/BackStackScreen$Companion;Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
231237
}
232238

233239
public final class com/squareup/workflow1/ui/navigation/BackStackScreenKt {
234240
public static final fun plus (Lcom/squareup/workflow1/ui/navigation/BackStackScreen;Lcom/squareup/workflow1/ui/navigation/BackStackScreen;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
235-
public static final fun toBackStackScreen (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
236-
public static final fun toBackStackScreenOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
241+
public static final fun toBackStackScreen (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
242+
public static synthetic fun toBackStackScreen$default (Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
243+
public static final fun toBackStackScreenOrNull (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
244+
public static synthetic fun toBackStackScreenOrNull$default (Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
237245
}
238246

239247
public final class com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Screen {

‎workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BackStackScreen.kt

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.squareup.workflow1.ui.navigation
22

3+
import com.squareup.workflow1.ui.Compatible
4+
import com.squareup.workflow1.ui.Compatible.Companion.keyFor
35
import com.squareup.workflow1.ui.Container
46
import com.squareup.workflow1.ui.Screen
57
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
@@ -18,18 +20,35 @@ import com.squareup.workflow1.ui.navigation.BackStackScreen.Companion.fromListOr
1820
*
1921
* @see fromList
2022
* @see fromListOrNull
23+
*
24+
* @param name included in the [compatibilityKey] of this screen, for ease
25+
* of composition -- in classic Android views, view state persistence support
26+
* requires peer BackStackScreens to have a unique keys.
2127
*/
2228
@WorkflowUiExperimentalApi
2329
public class BackStackScreen<out StackedT : Screen> internal constructor(
24-
public val frames: List<StackedT>
25-
) : Screen, Container<Screen, StackedT> {
30+
public val frames: List<StackedT>,
31+
public val name: String
32+
) : Screen, Container<Screen, StackedT>, Compatible {
33+
2634
/**
27-
* Creates a screen with elements listed from the [bottom] to the top.
35+
* Creates a [BackStackScreen] with elements listed from the [bottom] to the top.
2836
*/
2937
public constructor(
3038
bottom: StackedT,
3139
vararg rest: StackedT
32-
) : this(listOf(bottom) + rest)
40+
) : this(listOf(bottom) + rest, "")
41+
42+
/**
43+
* Creates a [named][name] [BackStackScreen] with elements listed from the [bottom] to the top.
44+
*/
45+
public constructor(
46+
name: String,
47+
bottom: StackedT,
48+
vararg rest: StackedT
49+
) : this(listOf(bottom) + rest, name)
50+
51+
override val compatibilityKey: String = keyFor(this, name)
3352

3453
override fun asSequence(): Sequence<StackedT> = frames.asSequence()
3554

@@ -48,24 +67,34 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
4867
public override fun <StackedU : Screen> map(
4968
transform: (StackedT) -> StackedU
5069
): BackStackScreen<StackedU> {
51-
return frames.map(transform).toBackStackScreen()
70+
return frames.map(transform).toBackStackScreen(name)
5271
}
5372

5473
public fun <R : Screen> mapIndexed(transform: (index: Int, StackedT) -> R): BackStackScreen<R> {
5574
return frames.mapIndexed(transform)
56-
.toBackStackScreen()
75+
.toBackStackScreen(name)
5776
}
5877

59-
override fun equals(other: Any?): Boolean {
60-
return (other as? BackStackScreen<*>)?.frames == frames
78+
public fun withName(name: String): BackStackScreen<StackedT> = BackStackScreen(frames, name)
79+
80+
override fun toString(): String {
81+
return name.takeIf { it.isNotEmpty() }?.let { "BackStackScreen-$it($frames)" }
82+
?: "BackStackScreen($frames)"
6183
}
6284

63-
override fun hashCode(): Int {
64-
return frames.hashCode()
85+
override fun equals(other: Any?): Boolean {
86+
if (this === other) return true
87+
if (javaClass != other?.javaClass) return false
88+
89+
other as BackStackScreen<*>
90+
91+
return (frames == other.frames && name == other.name)
6592
}
6693

67-
override fun toString(): String {
68-
return "${this::class.java.simpleName}($frames)"
94+
override fun hashCode(): Int {
95+
var result = frames.hashCode()
96+
result = 31 * result + name.hashCode()
97+
return result
6998
}
7099

71100
public companion object {
@@ -74,21 +103,27 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
74103
*
75104
* @throws IllegalArgumentException is [frames] is empty
76105
*/
77-
public fun <T : Screen> fromList(frames: List<T>): BackStackScreen<T> {
106+
public fun <T : Screen> fromList(
107+
frames: List<T>,
108+
name: String = ""
109+
): BackStackScreen<T> {
78110
require(frames.isNotEmpty()) {
79111
"A BackStackScreen must have at least one frame."
80112
}
81-
return BackStackScreen(frames)
113+
return BackStackScreen(frames, name)
82114
}
83115

84116
/**
85117
* Builds a [BackStackScreen] from a list of [frames], or returns `null`
86118
* if [frames] is empty.
87119
*/
88-
public fun <T : Screen> fromListOrNull(frames: List<T>): BackStackScreen<T>? {
120+
public fun <T : Screen> fromListOrNull(
121+
frames: List<T>,
122+
name: String = ""
123+
): BackStackScreen<T>? {
89124
return when {
90125
frames.isEmpty() -> null
91-
else -> BackStackScreen(frames)
126+
else -> BackStackScreen(frames, name)
92127
}
93128
}
94129
}
@@ -103,13 +138,13 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
103138
public operator fun <T : Screen> BackStackScreen<T>.plus(
104139
other: BackStackScreen<T>?
105140
): BackStackScreen<T> {
106-
return other?.let { BackStackScreen(frames + it.frames) } ?: this
141+
return other?.let { BackStackScreen(frames + it.frames, this.name) } ?: this
107142
}
108143

109144
@WorkflowUiExperimentalApi
110-
public fun <T : Screen> List<T>.toBackStackScreenOrNull(): BackStackScreen<T>? =
111-
fromListOrNull(this)
145+
public fun <T : Screen> List<T>.toBackStackScreenOrNull(name: String = ""): BackStackScreen<T>? =
146+
fromListOrNull(this, name)
112147

113148
@WorkflowUiExperimentalApi
114-
public fun <T : Screen> List<T>.toBackStackScreen(): BackStackScreen<T> =
115-
Companion.fromList(this)
149+
public fun <T : Screen> List<T>.toBackStackScreen(name: String = ""): BackStackScreen<T> =
150+
Companion.fromList(this, name)

‎workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
6969
* a [BodyAndOverlaysScreen] rendering.
7070
*
7171
* @param name included in the [compatibilityKey] of this screen, for ease
72-
* of nesting -- on Android, view state persistence support requires each
73-
* BodyAndOverlaysScreen in a hierarchy to have a unique key
72+
* of nesting -- in classic Android views, view state persistence support requires each
73+
* BodyAndOverlaysScreen in a hierarchy to have a unique key.
7474
*/
7575
@WorkflowUiExperimentalApi
7676
public class BodyAndOverlaysScreen<B : Screen, O : Overlay>(

‎workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/navigation/BackStackScreenTest.kt

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,30 @@ internal class BackStackScreenTest {
3333
}
3434

3535
@Test fun `plus another stack`() {
36-
assertThat(
37-
BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)) + BackStackScreen(
36+
val sum = BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)) +
37+
BackStackScreen("barney", FooScreen(8), FooScreen(9), FooScreen(0))
38+
assertThat(sum).isEqualTo(
39+
BackStackScreen(
40+
name = "fred",
41+
FooScreen(1),
42+
FooScreen(2),
43+
FooScreen(3),
3844
FooScreen(8),
3945
FooScreen(9),
4046
FooScreen(0)
4147
)
4248
)
43-
.isEqualTo(
44-
BackStackScreen(
45-
FooScreen(1),
46-
FooScreen(2),
47-
FooScreen(3),
48-
FooScreen(8),
49-
FooScreen(9),
50-
FooScreen(0)
51-
)
49+
assertThat(sum).isNotEqualTo(
50+
BackStackScreen(
51+
name = "barney",
52+
FooScreen(1),
53+
FooScreen(2),
54+
FooScreen(3),
55+
FooScreen(8),
56+
FooScreen(9),
57+
FooScreen(0)
5258
)
59+
)
5360
}
5461

5562
@Test fun `unequal by order`() {
@@ -70,9 +77,10 @@ internal class BackStackScreenTest {
7077
@Test fun `bottom and rest`() {
7178
assertThat(
7279
BackStackScreen.fromList(
73-
listOf(element = FooScreen(1)) + listOf(FooScreen(2), FooScreen(3), FooScreen(4))
80+
listOf(element = FooScreen(1)) + listOf(FooScreen(2), FooScreen(3), FooScreen(4)),
81+
"fred"
7482
)
75-
).isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3), FooScreen(4)))
83+
).isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3), FooScreen(4)))
7684
}
7785

7886
@Test fun singleton() {
@@ -84,18 +92,24 @@ internal class BackStackScreenTest {
8492

8593
@Test fun map() {
8694
assertThat(
87-
BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)).map {
95+
BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)).map {
8896
FooScreen(it.value * 2)
8997
}
9098
)
91-
.isEqualTo(BackStackScreen(FooScreen(2), FooScreen(4), FooScreen(6)))
99+
.isEqualTo(BackStackScreen("fred", FooScreen(2), FooScreen(4), FooScreen(6)))
92100
}
93101

94102
@Test fun mapIndexed() {
95-
val source = BackStackScreen(FooScreen("able"), FooScreen("baker"), FooScreen("charlie"))
103+
val source =
104+
BackStackScreen("fred", FooScreen("able"), FooScreen("baker"), FooScreen("charlie"))
96105
assertThat(source.mapIndexed { index, frame -> FooScreen("$index: ${frame.value}") })
97106
.isEqualTo(
98-
BackStackScreen(FooScreen("0: able"), FooScreen("1: baker"), FooScreen("2: charlie"))
107+
BackStackScreen(
108+
"fred",
109+
FooScreen("0: able"),
110+
FooScreen("1: baker"),
111+
FooScreen("2: charlie")
112+
)
99113
)
100114
}
101115

@@ -108,23 +122,28 @@ internal class BackStackScreenTest {
108122
}
109123

110124
@Test fun fromList() {
111-
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreen())
112-
.isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)))
125+
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreen("fred"))
126+
.isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)))
113127
}
114128

115129
@Test fun fromListOrNull() {
116-
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreenOrNull())
117-
.isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)))
130+
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreenOrNull("fred"))
131+
.isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)))
118132
}
119133

120134
/**
121135
* To reminds us why we want the `out` in `BackStackScreen<out T : Screen>`.
122136
* Without this, using `BackStackScreen<*>` as `RenderingT` is not practical.
123137
*/
124-
@Test fun heterogenousPlusIsTolerable() {
138+
@Test fun heterogeneousPlusIsTolerable() {
125139
val foo = BackStackScreen(FooScreen(1))
126140
val bar = BackStackScreen(BarScreen(1))
127141
val both = foo + bar
128142
assertThat(both).isEqualTo(foo + bar)
129143
}
144+
145+
@Test fun withName() {
146+
assertThat(BackStackScreen("fred", FooScreen(1)).withName("barney"))
147+
.isEqualTo(BackStackScreen("barney", FooScreen(1)))
148+
}
130149
}

0 commit comments

Comments
 (0)