From 5d385ce5b613c382a68a7be19f070649b0d1c7cb Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Thu, 21 Sep 2023 10:07:31 -0400 Subject: [PATCH] Friendlier construction API for `BackStackScreen` * Deprecates the confusing `bottom: T, rest: List` constructor * Adds `fromList` and `fromListOrNull` factory functions to `BackStackScreen.Companion`, since no one ever finds the `List` extensions --- .../compose/ComposeViewTreeIntegrationTest.kt | 4 +- .../BackStackContainerLifecycleActivity.kt | 2 +- workflow-ui/core-common/api/core-common.api | 7 ++ .../workflow1/ui/container/BackStackScreen.kt | 64 ++++++++++++++----- .../ui/container/BackStackScreenTest.kt | 5 +- 5 files changed, 60 insertions(+), 22 deletions(-) diff --git a/workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/ComposeViewTreeIntegrationTest.kt b/workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/ComposeViewTreeIntegrationTest.kt index 6429fd1fb..5625bc776 100644 --- a/workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/ComposeViewTreeIntegrationTest.kt +++ b/workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/ComposeViewTreeIntegrationTest.kt @@ -582,7 +582,9 @@ internal class ComposeViewTreeIntegrationTest { } private fun WorkflowUiTestActivity.setBackstack(vararg backstack: TestComposeRendering) { - setRendering(BackStackScreen(EmptyRendering, backstack.asList())) + setRendering( + BackStackScreen.fromList(listOf>(EmptyRendering) + backstack.asList()) + ) } data class TestOverlay( diff --git a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/fixtures/BackStackContainerLifecycleActivity.kt b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/fixtures/BackStackContainerLifecycleActivity.kt index ec765f64b..c42829140 100644 --- a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/fixtures/BackStackContainerLifecycleActivity.kt +++ b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/fixtures/BackStackContainerLifecycleActivity.kt @@ -145,7 +145,7 @@ internal class BackStackContainerLifecycleActivity : AbstractLifecycleTestActivi setRendering(backstack.asList().toBackstackWithBase()) private fun List.toBackstackWithBase() = - BackStackScreen(BaseRendering, this) + BackStackScreen.fromList(listOf(BaseRendering) + this) } internal fun ActivityScenario.viewForScreen( diff --git a/workflow-ui/core-common/api/core-common.api b/workflow-ui/core-common/api/core-common.api index e8d392eb0..d0fa3c05e 100644 --- a/workflow-ui/core-common/api/core-common.api +++ b/workflow-ui/core-common/api/core-common.api @@ -213,8 +213,10 @@ public final class com/squareup/workflow1/ui/container/BackStackConfigKt { } public final class com/squareup/workflow1/ui/container/BackStackScreen : com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen { + public static final field Companion Lcom/squareup/workflow1/ui/container/BackStackScreen$Companion; public fun (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;)V public fun (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V + public synthetic fun (Ljava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public fun asSequence ()Lkotlin/sequences/Sequence; public fun equals (Ljava/lang/Object;)Z public final fun get (I)Lcom/squareup/workflow1/ui/Screen; @@ -229,6 +231,11 @@ public final class com/squareup/workflow1/ui/container/BackStackScreen : com/squ public fun toString ()Ljava/lang/String; } +public final class com/squareup/workflow1/ui/container/BackStackScreen$Companion { + public final fun fromList (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen; + public final fun fromListOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen; +} + public final class com/squareup/workflow1/ui/container/BackStackScreenKt { public static final fun toBackStackScreen (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen; public static final fun toBackStackScreenOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen; diff --git a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/container/BackStackScreen.kt b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/container/BackStackScreen.kt index bb8815eee..fc6199171 100644 --- a/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/container/BackStackScreen.kt +++ b/workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/container/BackStackScreen.kt @@ -3,6 +3,9 @@ package com.squareup.workflow1.ui.container import com.squareup.workflow1.ui.Container import com.squareup.workflow1.ui.Screen import com.squareup.workflow1.ui.WorkflowUiExperimentalApi +import com.squareup.workflow1.ui.container.BackStackScreen.Companion +import com.squareup.workflow1.ui.container.BackStackScreen.Companion.fromList +import com.squareup.workflow1.ui.container.BackStackScreen.Companion.fromListOrNull /** * Represents an active screen ([top]), and a set of previously visited screens to which we may @@ -13,13 +16,12 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi * * UI kits are expected to provide handling for this class by default. * - * @param bottom the bottom-most entry in the stack - * @param rest the rest of the stack, empty by default + * @see fromList + * @see fromListOrNull */ @WorkflowUiExperimentalApi -public class BackStackScreen( - bottom: StackedT, - rest: List +public class BackStackScreen private constructor( + public val frames: List ) : Screen, Container { /** * Creates a screen with elements listed from the [bottom] to the top. @@ -27,11 +29,18 @@ public class BackStackScreen( public constructor( bottom: StackedT, vararg rest: StackedT - ) : this(bottom, rest.toList()) + ) : this(listOf(bottom) + rest) - override fun asSequence(): Sequence = frames.asSequence() + @Deprecated( + "Use fromList", + ReplaceWith("BackStackScreen.fromList(listOf(bottom) + rest)") + ) + public constructor( + bottom: StackedT, + rest: List + ) : this(listOf(bottom) + rest) - public val frames: List = listOf(bottom) + rest + override fun asSequence(): Sequence = frames.asSequence() /** * The active screen. @@ -49,7 +58,7 @@ public class BackStackScreen( return if (other == null) { this } else { - BackStackScreen(frames[0], frames.subList(1, frames.size) + other.frames) + BackStackScreen(frames + other.frames) } } @@ -75,16 +84,37 @@ public class BackStackScreen( override fun toString(): String { return "${this::class.java.simpleName}($frames)" } + + public companion object { + /** + * Builds a [BackStackScreen] from a non-empty list of [frames]. + * + * @throws IllegalArgumentException is [frames] is empty + */ + public fun fromList(frames: List): BackStackScreen { + require(frames.isNotEmpty()) { + "A BackStackScreen must have at least one frame." + } + return BackStackScreen(frames) + } + + /** + * Builds a [BackStackScreen] from a list of [frames], or returns `null` + * if [frames] is empty. + */ + public fun fromListOrNull(frames: List): BackStackScreen? { + return when { + frames.isEmpty() -> null + else -> BackStackScreen(frames) + } + } + } } @WorkflowUiExperimentalApi -public fun List.toBackStackScreenOrNull(): BackStackScreen? = when { - isEmpty() -> null - else -> toBackStackScreen() -} +public fun List.toBackStackScreenOrNull(): BackStackScreen? = + fromListOrNull(this) @WorkflowUiExperimentalApi -public fun List.toBackStackScreen(): BackStackScreen { - require(isNotEmpty()) - return BackStackScreen(first(), subList(1, size)) -} +public fun List.toBackStackScreen(): BackStackScreen = + Companion.fromList(this) diff --git a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/container/BackStackScreenTest.kt b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/container/BackStackScreenTest.kt index 936f1fb17..9d9daff82 100644 --- a/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/container/BackStackScreenTest.kt +++ b/workflow-ui/core-common/src/test/java/com/squareup/workflow1/ui/container/BackStackScreenTest.kt @@ -45,9 +45,8 @@ internal class BackStackScreenTest { @Test fun `bottom and rest`() { assertThat( - BackStackScreen( - bottom = S(1), - rest = listOf(S(2), S(3), S(4)) + BackStackScreen.fromList( + listOf(element = S(1)) + listOf(S(2), S(3), S(4)) ) ).isEqualTo(BackStackScreen(S(1), S(2), S(3), S(4))) }