From fad5aa5fabbfcfbd03bb4cd9d15d37170a2c05f0 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Thu, 12 Sep 2024 17:22:38 -0700 Subject: [PATCH] Introduces `BackStackScreen.name` `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`. --- .../overviewdetail/OverviewDetailContainer.kt | 3 +- .../WorkflowSavedStateRegistryAggregator.kt | 3 +- workflow-ui/core-common/api/core-common.api | 17 ++-- .../ui/navigation/BackStackScreen.kt | 77 ++++++++++++++----- .../ui/navigation/BodyAndOverlaysScreen.kt | 4 +- .../ui/navigation/BackStackScreenTest.kt | 63 +++++++++------ 6 files changed, 114 insertions(+), 53 deletions(-) diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt index 03267580b..f829fc570 100644 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt +++ b/samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt @@ -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 @@ -64,7 +63,7 @@ class OverviewDetailContainer(view: View) : ScreenViewRunner (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V + public fun (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; @@ -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 { diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BackStackScreen.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BackStackScreen.kt index 6b3d4c14c..85d4dab12 100644 --- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BackStackScreen.kt +++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BackStackScreen.kt @@ -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 @@ -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 internal constructor( - public val frames: List -) : Screen, Container { + public val frames: List, + public val name: String +) : Screen, Container, 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 = frames.asSequence() @@ -48,24 +67,34 @@ public class BackStackScreen internal constructor( public override fun map( transform: (StackedT) -> StackedU ): BackStackScreen { - return frames.map(transform).toBackStackScreen() + return frames.map(transform).toBackStackScreen(name) } public fun mapIndexed(transform: (index: Int, StackedT) -> R): BackStackScreen { 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 = 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 { @@ -74,21 +103,27 @@ public class BackStackScreen internal constructor( * * @throws IllegalArgumentException is [frames] is empty */ - public fun fromList(frames: List): BackStackScreen { + public fun fromList( + frames: List, + name: String = "" + ): BackStackScreen { 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 fromListOrNull(frames: List): BackStackScreen? { + public fun fromListOrNull( + frames: List, + name: String = "" + ): BackStackScreen? { return when { frames.isEmpty() -> null - else -> BackStackScreen(frames) + else -> BackStackScreen(frames, name) } } } @@ -103,13 +138,13 @@ public class BackStackScreen internal constructor( public operator fun BackStackScreen.plus( other: BackStackScreen? ): BackStackScreen { - return other?.let { BackStackScreen(frames + it.frames) } ?: this + return other?.let { BackStackScreen(frames + it.frames, this.name) } ?: this } @WorkflowUiExperimentalApi -public fun List.toBackStackScreenOrNull(): BackStackScreen? = - fromListOrNull(this) +public fun List.toBackStackScreenOrNull(name: String = ""): BackStackScreen? = + fromListOrNull(this, name) @WorkflowUiExperimentalApi -public fun List.toBackStackScreen(): BackStackScreen = - Companion.fromList(this) +public fun List.toBackStackScreen(name: String = ""): BackStackScreen = + Companion.fromList(this, name) diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen.kt index db22e4af3..cd1b7a4a6 100644 --- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen.kt +++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/navigation/BodyAndOverlaysScreen.kt @@ -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( diff --git a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/navigation/BackStackScreenTest.kt b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/navigation/BackStackScreenTest.kt index 2760a67cf..da825497a 100644 --- a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/navigation/BackStackScreenTest.kt +++ b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/navigation/BackStackScreenTest.kt @@ -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`() { @@ -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() { @@ -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") + ) ) } @@ -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`. * 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))) + } }