Skip to content

Commit

Permalink
Introduces BackStackScreen.name
Browse files Browse the repository at this point in the history
`BackStackScreen` now implements `Compatible` and has a `name: String` field, just like `BodyAndOverlaysScreen`. This is in support of having multiple instances under a single `SavedStateRegistry` at a time. Our old trick of wrapping instances via `NamedScreen` doesn't work the the `BackStackScreen` creates the first classic `View` instance in Compose context, because the `NamedScreen` will be handled by Compose code. The old hack in `BackStackContainer` relies on `View.screen` holdling `NamedScreen(BackStackScreen)`, but in the Compose-eats-NamedScreen case it will just be `BackStackScreen`.
  • Loading branch information
rjrjr committed Sep 13, 2024
1 parent d4cc5b4 commit fad5aa5
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import com.squareup.sample.container.R
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Detail
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Overview
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Single
import com.squareup.workflow1.ui.NamedScreen
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
Expand Down Expand Up @@ -64,7 +63,7 @@ class OverviewDetailContainer(view: View) : ScreenViewRunner<OverviewDetailScree

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

rendering.detailRendering
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ public class WorkflowSavedStateRegistryAggregator {
"Error registering SavedStateProvider: key \"$key\" is already in " +
"use on parent SavedStateRegistryOwner $parentOwner. " +
"This is most easily remedied by giving your container Screen rendering a unique " +
"Compatible.compatibilityKey, perhaps by wrapping it with Named.",
"Compatible.compatibilityKey -- note the name fields on BodyAndOverlaysScreen " +
"and BackStackScreen.",
e
)
}
Expand Down
17 changes: 12 additions & 5 deletions workflow-ui/core-common/api/core-common.api
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,17 @@ public final class com/squareup/workflow1/ui/navigation/BackStackConfigKt {
public static final fun plus (Lcom/squareup/workflow1/ui/ViewEnvironment;Lcom/squareup/workflow1/ui/navigation/BackStackConfig;)Lcom/squareup/workflow1/ui/ViewEnvironment;
}

public final class com/squareup/workflow1/ui/navigation/BackStackScreen : com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen {
public final class com/squareup/workflow1/ui/navigation/BackStackScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen {
public static final field Companion Lcom/squareup/workflow1/ui/navigation/BackStackScreen$Companion;
public fun <init> (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V
public fun <init> (Ljava/lang/String;Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V
public fun asSequence ()Lkotlin/sequences/Sequence;
public fun equals (Ljava/lang/Object;)Z
public final fun get (I)Lcom/squareup/workflow1/ui/Screen;
public final fun getBackStack ()Ljava/util/List;
public fun getCompatibilityKey ()Ljava/lang/String;
public final fun getFrames ()Ljava/util/List;
public final fun getName ()Ljava/lang/String;
public final fun getTop ()Lcom/squareup/workflow1/ui/Screen;
public fun hashCode ()I
public synthetic fun map (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/Container;
Expand All @@ -226,14 +229,18 @@ public final class com/squareup/workflow1/ui/navigation/BackStackScreen : com/sq
}

public final class com/squareup/workflow1/ui/navigation/BackStackScreen$Companion {
public final fun fromList (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public final fun fromListOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public final fun fromList (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
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;
public final fun fromListOrNull (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
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;
}

public final class com/squareup/workflow1/ui/navigation/BackStackScreenKt {
public static final fun plus (Lcom/squareup/workflow1/ui/navigation/BackStackScreen;Lcom/squareup/workflow1/ui/navigation/BackStackScreen;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static final fun toBackStackScreen (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static final fun toBackStackScreenOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static final fun toBackStackScreen (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static synthetic fun toBackStackScreen$default (Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static final fun toBackStackScreenOrNull (Ljava/util/List;Ljava/lang/String;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
public static synthetic fun toBackStackScreenOrNull$default (Ljava/util/List;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/navigation/BackStackScreen;
}

public final class com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Screen {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.squareup.workflow1.ui.navigation

import com.squareup.workflow1.ui.Compatible
import com.squareup.workflow1.ui.Compatible.Companion.keyFor
import com.squareup.workflow1.ui.Container
import com.squareup.workflow1.ui.Screen
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
Expand All @@ -18,18 +20,35 @@ import com.squareup.workflow1.ui.navigation.BackStackScreen.Companion.fromListOr
*
* @see fromList
* @see fromListOrNull
*
* @param name included in the [compatibilityKey] of this screen, for ease
* of composition -- in classic Android views, view state persistence support
* requires peer BackStackScreens to have a unique keys.
*/
@WorkflowUiExperimentalApi
public class BackStackScreen<out StackedT : Screen> internal constructor(
public val frames: List<StackedT>
) : Screen, Container<Screen, StackedT> {
public val frames: List<StackedT>,
public val name: String
) : Screen, Container<Screen, StackedT>, Compatible {

/**
* Creates a screen with elements listed from the [bottom] to the top.
* Creates a [BackStackScreen] with elements listed from the [bottom] to the top.
*/
public constructor(
bottom: StackedT,
vararg rest: StackedT
) : this(listOf(bottom) + rest)
) : this(listOf(bottom) + rest, "")

/**
* Creates a [named][name] [BackStackScreen] with elements listed from the [bottom] to the top.
*/
public constructor(
name: String,
bottom: StackedT,
vararg rest: StackedT
) : this(listOf(bottom) + rest, name)

override val compatibilityKey: String = keyFor(this, name)

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

Expand All @@ -48,24 +67,34 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
public override fun <StackedU : Screen> map(
transform: (StackedT) -> StackedU
): BackStackScreen<StackedU> {
return frames.map(transform).toBackStackScreen()
return frames.map(transform).toBackStackScreen(name)
}

public fun <R : Screen> mapIndexed(transform: (index: Int, StackedT) -> R): BackStackScreen<R> {
return frames.mapIndexed(transform)
.toBackStackScreen()
.toBackStackScreen(name)
}

override fun equals(other: Any?): Boolean {
return (other as? BackStackScreen<*>)?.frames == frames
public fun withName(name: String): BackStackScreen<StackedT> = BackStackScreen(frames, name)

override fun toString(): String {
return name.takeIf { it.isNotEmpty() }?.let { "BackStackScreen-$it($frames)" }
?: "BackStackScreen($frames)"
}

override fun hashCode(): Int {
return frames.hashCode()
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as BackStackScreen<*>

return (frames == other.frames && name == other.name)
}

override fun toString(): String {
return "${this::class.java.simpleName}($frames)"
override fun hashCode(): Int {
var result = frames.hashCode()
result = 31 * result + name.hashCode()
return result
}

public companion object {
Expand All @@ -74,21 +103,27 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
*
* @throws IllegalArgumentException is [frames] is empty
*/
public fun <T : Screen> fromList(frames: List<T>): BackStackScreen<T> {
public fun <T : Screen> fromList(
frames: List<T>,
name: String = ""
): BackStackScreen<T> {
require(frames.isNotEmpty()) {
"A BackStackScreen must have at least one frame."
}
return BackStackScreen(frames)
return BackStackScreen(frames, name)
}

/**
* Builds a [BackStackScreen] from a list of [frames], or returns `null`
* if [frames] is empty.
*/
public fun <T : Screen> fromListOrNull(frames: List<T>): BackStackScreen<T>? {
public fun <T : Screen> fromListOrNull(
frames: List<T>,
name: String = ""
): BackStackScreen<T>? {
return when {
frames.isEmpty() -> null
else -> BackStackScreen(frames)
else -> BackStackScreen(frames, name)
}
}
}
Expand All @@ -103,13 +138,13 @@ public class BackStackScreen<out StackedT : Screen> internal constructor(
public operator fun <T : Screen> BackStackScreen<T>.plus(
other: BackStackScreen<T>?
): BackStackScreen<T> {
return other?.let { BackStackScreen(frames + it.frames) } ?: this
return other?.let { BackStackScreen(frames + it.frames, this.name) } ?: this
}

@WorkflowUiExperimentalApi
public fun <T : Screen> List<T>.toBackStackScreenOrNull(): BackStackScreen<T>? =
fromListOrNull(this)
public fun <T : Screen> List<T>.toBackStackScreenOrNull(name: String = ""): BackStackScreen<T>? =
fromListOrNull(this, name)

@WorkflowUiExperimentalApi
public fun <T : Screen> List<T>.toBackStackScreen(): BackStackScreen<T> =
Companion.fromList(this)
public fun <T : Screen> List<T>.toBackStackScreen(name: String = ""): BackStackScreen<T> =
Companion.fromList(this, name)
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
* a [BodyAndOverlaysScreen] rendering.
*
* @param name included in the [compatibilityKey] of this screen, for ease
* of nesting -- on Android, view state persistence support requires each
* BodyAndOverlaysScreen in a hierarchy to have a unique key
* of nesting -- in classic Android views, view state persistence support requires each
* BodyAndOverlaysScreen in a hierarchy to have a unique key.
*/
@WorkflowUiExperimentalApi
public class BodyAndOverlaysScreen<B : Screen, O : Overlay>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,30 @@ internal class BackStackScreenTest {
}

@Test fun `plus another stack`() {
assertThat(
BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)) + BackStackScreen(
val sum = BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)) +
BackStackScreen("barney", FooScreen(8), FooScreen(9), FooScreen(0))
assertThat(sum).isEqualTo(
BackStackScreen(
name = "fred",
FooScreen(1),
FooScreen(2),
FooScreen(3),
FooScreen(8),
FooScreen(9),
FooScreen(0)
)
)
.isEqualTo(
BackStackScreen(
FooScreen(1),
FooScreen(2),
FooScreen(3),
FooScreen(8),
FooScreen(9),
FooScreen(0)
)
assertThat(sum).isNotEqualTo(
BackStackScreen(
name = "barney",
FooScreen(1),
FooScreen(2),
FooScreen(3),
FooScreen(8),
FooScreen(9),
FooScreen(0)
)
)
}

@Test fun `unequal by order`() {
Expand All @@ -70,9 +77,10 @@ internal class BackStackScreenTest {
@Test fun `bottom and rest`() {
assertThat(
BackStackScreen.fromList(
listOf(element = FooScreen(1)) + listOf(FooScreen(2), FooScreen(3), FooScreen(4))
listOf(element = FooScreen(1)) + listOf(FooScreen(2), FooScreen(3), FooScreen(4)),
"fred"
)
).isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3), FooScreen(4)))
).isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3), FooScreen(4)))
}

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

@Test fun map() {
assertThat(
BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)).map {
BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)).map {
FooScreen(it.value * 2)
}
)
.isEqualTo(BackStackScreen(FooScreen(2), FooScreen(4), FooScreen(6)))
.isEqualTo(BackStackScreen("fred", FooScreen(2), FooScreen(4), FooScreen(6)))
}

@Test fun mapIndexed() {
val source = BackStackScreen(FooScreen("able"), FooScreen("baker"), FooScreen("charlie"))
val source =
BackStackScreen("fred", FooScreen("able"), FooScreen("baker"), FooScreen("charlie"))
assertThat(source.mapIndexed { index, frame -> FooScreen("$index: ${frame.value}") })
.isEqualTo(
BackStackScreen(FooScreen("0: able"), FooScreen("1: baker"), FooScreen("2: charlie"))
BackStackScreen(
"fred",
FooScreen("0: able"),
FooScreen("1: baker"),
FooScreen("2: charlie")
)
)
}

Expand All @@ -108,23 +122,28 @@ internal class BackStackScreenTest {
}

@Test fun fromList() {
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreen())
.isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)))
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreen("fred"))
.isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)))
}

@Test fun fromListOrNull() {
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreenOrNull())
.isEqualTo(BackStackScreen(FooScreen(1), FooScreen(2), FooScreen(3)))
assertThat(listOf(FooScreen(1), FooScreen(2), FooScreen(3)).toBackStackScreenOrNull("fred"))
.isEqualTo(BackStackScreen("fred", FooScreen(1), FooScreen(2), FooScreen(3)))
}

/**
* To reminds us why we want the `out` in `BackStackScreen<out T : Screen>`.
* Without this, using `BackStackScreen<*>` as `RenderingT` is not practical.
*/
@Test fun heterogenousPlusIsTolerable() {
@Test fun heterogeneousPlusIsTolerable() {
val foo = BackStackScreen(FooScreen(1))
val bar = BackStackScreen(BarScreen(1))
val both = foo + bar
assertThat(both).isEqualTo(foo + bar)
}

@Test fun withName() {
assertThat(BackStackScreen("fred", FooScreen(1)).withName("barney"))
.isEqualTo(BackStackScreen("barney", FooScreen(1)))
}
}

0 comments on commit fad5aa5

Please sign in to comment.