API overhaul because splitting the keys and the composables don't work.

All of our tests and demos were built using `String` as the key, with `content` that does nothing but render the key. This approach doesn't reflect reality very well, and masked #63, where keys for more interesting objects can get out of sync with the `content` lambda that can render them. When popping, you would wind up crashing when the up to date lambda is unable to interpret the key for the screen that is being animated away. The fix is to change the API from something that takes a list of keys and a function that can render them, to a list of model objects that themselves are able to provide `@Composable Content()`. IMHO the updated API actually feels pretty good, more like the conventional hoisted-state `@Composable Foo(model: FooModel)` idiom. (Of course I've been working on this all day, so I'm biased.) We provide a new interface: ```kotlin interface BackstackFrame<out K : Any> { val key: K @composable fun Content() } ``` And change the signature of the `Backstack()` function: ```kotlin fun <K : Any> Backstack( frames: List<BackstackFrame<K>>, modifier: Modifier = Modifier, frameController: FrameController<K> ) ``` Note that the param type, `K`, is still the type of the key, not the type of a particular flavor of `BackstackFrame`. This makes it easy for us to provide convenience functions to map lists of arbitrary model objects to `BackstackFrame` instances, so it's not much more verbose than it used to be to make it go. Before: ```kotlin Backstack(backstack) { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } } ``` After: ```kotlin Backstack( backstack.toBackstackModel { screen -> when(screen) { Screen.ContactList -> ShowContactList(navigator) is Screen.ContactDetails -> ShowContact(screen.id, navigator) is Screen.EditContact -> ShowEditContact(screen.id, navigator) } ) ``` Note that there are two flavors of `toBackstackModel`. The second one supports models with more interesting keys. ```kotlin data class Portrait( val id: Int, val url: String ) Backstack( backstack.toBackstackModel( getKey = { it.id } ) { PrettyPicture(it) } ) ``` Fixes #63
rjrjr · Aug 27, 2022 · 71d36fb · 71d36fb
1 parent 82429b4
commit 71d36fb
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 150 deletions.
diff --git a/buildSrc/src/main/resources/versions.properties b/buildSrc/src/main/resources/versions.properties
@@ -1,3 +1,3 @@
 # -SNAPSHOT will automatically be appended. Pass -PisRelease=true to gradlew to release (this will
 # also append the current compose version number after a +).
-releaseVersion=0.10.0
+releaseVersion=0.11.0
diff --git a/...ckstack-viewer/src/main/java/com/zachklipp/compose/backstack/viewer/BackstackViewerApp.kt b/...ckstack-viewer/src/main/java/com/zachklipp/compose/backstack/viewer/BackstackViewerApp.kt
@@ -41,6 +41,7 @@ import com.zachklipp.compose.backstack.BackstackTransition.Crossfade
 import com.zachklipp.compose.backstack.BackstackTransition.Slide
 import com.zachklipp.compose.backstack.defaultBackstackAnimation
 import com.zachklipp.compose.backstack.rememberTransitionController
+import com.zachklipp.compose.backstack.toBackstackModel
 import com.zachklipp.compose.backstack.xray.xrayed
 
 private val DEFAULT_BACKSTACKS = listOf(
@@ -147,7 +148,14 @@ private fun AppScreens(model: AppModel) {
 
   MaterialTheme(colors = lightColors()) {
     Backstack(
-      backstack = model.currentBackstack,
+      frames = model.currentBackstack.toBackstackModel { screen ->
+        AppScreen(
+          name = screen,
+          showBack = screen != model.bottomScreen,
+          onAdd = { model.pushScreen("$screen+") },
+          onBack = model::popScreen
+        )
+      },
       frameController = rememberTransitionController<String>(
         transition = model.selectedTransition.second,
         animationSpec = animation ?: defaultBackstackAnimation(),
@@ -165,14 +173,7 @@ private fun AppScreens(model: AppModel) {
       modifier = Modifier
         .fillMaxSize()
         .border(width = 3.dp, color = Color.Red),
-    ) { screen ->
-      AppScreen(
-        name = screen,
-        showBack = screen != model.bottomScreen,
-        onAdd = { model.pushScreen("$screen+") },
-        onBack = model::popScreen
-      )
-    }
+    )
   }
 }
 

diff --git a/compose-backstack-xray/src/main/java/com/zachklipp/compose/backstack/xray/XrayController.kt b/compose-backstack-xray/src/main/java/com/zachklipp/compose/backstack/xray/XrayController.kt
@@ -12,8 +12,9 @@ import androidx.compose.ui.graphics.DefaultCameraDistance
 import androidx.compose.ui.graphics.graphicsLayer
 import androidx.compose.ui.input.pointer.pointerInput
 import androidx.compose.ui.unit.dp
+import com.zachklipp.compose.backstack.BackstackFrame
 import com.zachklipp.compose.backstack.FrameController
-import com.zachklipp.compose.backstack.FrameController.BackstackFrame
+import com.zachklipp.compose.backstack.FrameController.FrameAndModifier
 import com.zachklipp.compose.backstack.NoopFrameController
 import kotlin.math.sin
 
@@ -22,16 +23,16 @@ import kotlin.math.sin
  * the screens in the backstack in pseudo-3D space. The 3D stack can be navigated via touch
  * gestures.
  */
-@Composable fun <T : Any> FrameController<T>.xrayed(enabled: Boolean): FrameController<T> =
-  remember { XrayController<T>() }.also {
+@Composable fun <K : Any> FrameController<K>.xrayed(enabled: Boolean): FrameController<K> =
+  remember { XrayController<K>() }.also {
     it.enabled = enabled
     it.wrappedController = this
   }
 
-private class XrayController<T : Any> : FrameController<T> {
+private class XrayController<K : Any> : FrameController<K> {
 
   var enabled: Boolean by mutableStateOf(false)
-  var wrappedController: FrameController<T> by mutableStateOf(NoopFrameController())
+  var wrappedController: FrameController<K> by mutableStateOf(NoopFrameController())
 
   private var offsetDpX by mutableStateOf(500.dp)
   private var offsetDpY by mutableStateOf(10.dp)
@@ -41,7 +42,7 @@ private class XrayController<T : Any> : FrameController<T> {
   private var alpha by mutableStateOf(.4f)
   private var overlayAlpha by mutableStateOf(.2f)
 
-  private var activeKeys by mutableStateOf(emptyList<T>())
+  private var activeKeys by mutableStateOf(emptyList<BackstackFrame<K>>())
 
   private val controlModifier = Modifier.pointerInput(Unit) {
     detectTransformGestures { _, pan, zoom, _ ->
@@ -56,14 +57,14 @@ private class XrayController<T : Any> : FrameController<T> {
     if (!enabled) wrappedController.activeFrames else {
       activeKeys.mapIndexed { index, key ->
         val modifier = Modifier.modifierForFrame(index, activeKeys.size, 1f)
-        return@mapIndexed BackstackFrame(key, modifier)
+        return@mapIndexed FrameAndModifier(key, modifier)
       }
     }
   }
 
-  override fun updateBackstack(keys: List<T>) {
-    activeKeys = keys
-    wrappedController.updateBackstack(keys)
+  override fun updateBackstack(frames: List<BackstackFrame<K>>) {
+    activeKeys = frames
+    wrappedController.updateBackstack(frames)
   }
 
   /**

diff --git a/compose-backstack/src/androidTest/java/com/zachklipp/compose/backstack/BackstackStateTest.kt b/compose-backstack/src/androidTest/java/com/zachklipp/compose/backstack/BackstackStateTest.kt
@@ -10,11 +10,9 @@ import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.test.assertIsDisplayed
-import androidx.compose.ui.test.junit4.AndroidComposeTestRule
 import androidx.compose.ui.test.junit4.createComposeRule
 import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
-import androidx.test.ext.junit.rules.ActivityScenarioRule
 import com.google.common.truth.Truth.assertThat
 import org.junit.Rule
 import org.junit.Test
@@ -24,13 +22,15 @@ class BackstackStateTest {
   @get:Rule
   val compose = createComposeRule()
 
+  private fun List<String>.toCounters() = toBackstackModel {
+    var counter by rememberSaveable { mutableStateOf(0) }
+    BasicText("$it: $counter", Modifier.clickable { counter++ })
+  }
+
   @Test fun screen_state_is_restored_on_pop() {
     val backstack = mutableStateListOf("one")
     compose.setContent {
-      Backstack(backstack, frameController = NoopFrameController()) {
-        var counter by rememberSaveable { mutableStateOf(0) }
-        BasicText("$it: $counter", Modifier.clickable { counter++ })
-      }
+      Backstack(backstack.toCounters(), frameController = NoopFrameController())
     }
 
     // Update some state on the first screen.
@@ -55,10 +55,7 @@ class BackstackStateTest {
   @Test fun screen_state_is_discarded_after_pop() {
     val backstack = mutableStateListOf("one", "two")
     compose.setContent {
-      Backstack(backstack, frameController = NoopFrameController()) {
-        var counter by rememberSaveable { mutableStateOf(0) }
-        BasicText("$it: $counter", Modifier.clickable { counter++ })
-      }
+      Backstack(backstack.toCounters(), frameController = NoopFrameController())
     }
 
     // Update some state on the second screen.
@@ -78,10 +75,7 @@ class BackstackStateTest {
   @Test fun screen_state_is_discarded_when_removed_from_backstack_while_hidden() {
     var backstack by mutableStateOf(listOf("one"))
     compose.setContent {
-      Backstack(backstack, frameController = NoopFrameController()) {
-        var counter by rememberSaveable { mutableStateOf(0) }
-        BasicText("$it: $counter", Modifier.clickable { counter++ })
-      }
+      Backstack(backstack.toCounters(), frameController = NoopFrameController())
     }
 
     // Update some state on the first screen.
@@ -112,13 +106,16 @@ class BackstackStateTest {
     val backstack = mutableStateListOf("one")
     val transcript = mutableListOf<String>()
     compose.setContent {
-      Backstack(backstack, frameController = NoopFrameController()) {
-        BasicText(it)
-        DisposableEffect(Unit) {
-          transcript += "+$it"
-          onDispose { transcript += "-$it" }
-        }
-      }
+      Backstack(
+        backstack.toBackstackModel {
+          BasicText(it)
+          DisposableEffect(Unit) {
+            transcript += "+$it"
+            onDispose { transcript += "-$it" }
+          }
+        },
+        frameController = NoopFrameController()
+      )
     }
 
     assertThat(transcript).containsExactly("+one")
@@ -143,10 +140,13 @@ class BackstackStateTest {
 
     val backstack = mutableStateListOf(Screen("one"))
     compose.setContent {
-      Backstack(backstack, frameController = NoopFrameController()) {
-        var counter by rememberSaveable { mutableStateOf(0) }
-        BasicText("${it.name}: $counter", Modifier.clickable { counter++ })
-      }
+      Backstack(
+        backstack.toBackstackModel {
+          var counter by rememberSaveable { mutableStateOf(0) }
+          BasicText("${it.name}: $counter", Modifier.clickable { counter++ })
+        },
+        frameController = NoopFrameController()
+      )
     }
 
     // Update some state on the first screen.

diff --git a/...ackstack/src/androidTest/java/com/zachklipp/compose/backstack/BackstackTransitionsTest.kt b/...ackstack/src/androidTest/java/com/zachklipp/compose/backstack/BackstackTransitionsTest.kt
@@ -63,10 +63,12 @@ class BackstackTransitionsTest {
     assertTransition(Crossfade, forward = false)
   }
 
+  private fun List<String>.toBackstack() = toBackstackModel { BasicText(it) }
+
   private fun assertInitialStateWithSingleScreen(transition: BackstackTransition) {
     val originalBackstack = listOf("one")
     compose.setContent {
-      Backstack(originalBackstack, transition = transition) { BasicText(it) }
+      Backstack(originalBackstack.toBackstack(), transition = transition)
     }
 
     compose.onNodeWithText("one").assertIsDisplayed()
@@ -75,7 +77,7 @@ class BackstackTransitionsTest {
   private fun assertInitialStateWithMultipleScreens(transition: BackstackTransition) {
     val originalBackstack = listOf("one", "two")
     compose.setContent {
-      Backstack(originalBackstack, transition = transition) { BasicText(it) }
+      Backstack(originalBackstack.toBackstack(), transition = transition)
     }
 
     compose.onNodeWithText("two").assertIsDisplayed()
@@ -87,15 +89,17 @@ class BackstackTransitionsTest {
     val secondBackstack = listOf("one", "two")
     var backstack by mutableStateOf(if (forward) firstBackstack else secondBackstack)
     compose.mainClock.autoAdvance = false
+
     compose.setContent {
       Backstack(
-        backstack,
+        backstack.toBackstack(),
         frameController = rememberTransitionController(
           animationSpec = animation,
           transition = transition
         )
-      ) { BasicText(it) }
+      )
     }
+
     val initialText = if (forward) "one" else "two"
     val newText = if (forward) "two" else "one"