Skip to content

Commit

Permalink
API overhaul because splitting the keys and the composables don't work.
Browse files Browse the repository at this point in the history
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.url)
  }
)
```

Fixes #63
  • Loading branch information
rjrjr committed Aug 27, 2022
1 parent 82429b4 commit 8b55940
Show file tree
Hide file tree
Showing 11 changed files with 205 additions and 165 deletions.
2 changes: 1 addition & 1 deletion buildSrc/src/main/resources/versions.properties
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
Expand All @@ -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
)
}
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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, _ ->
Expand All @@ -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)
}

/**
Expand Down
25 changes: 14 additions & 11 deletions compose-backstack/api/compose-backstack.api
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
public abstract interface class com/zachklipp/compose/backstack/BackstackFrame {
public abstract fun Content (Landroidx/compose/runtime/Composer;I)V
public abstract fun getKey ()Ljava/lang/Object;
}

public final class com/zachklipp/compose/backstack/BackstackKt {
public static final fun Backstack (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/BackstackTransition;Landroidx/compose/animation/core/AnimationSpec;Lkotlin/jvm/functions/Function3;Lkotlin/jvm/functions/Function0;Ljava/lang/Object;Lkotlin/jvm/functions/Function3;)V
public static final fun Backstack (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/BackstackTransition;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V
public static final fun Backstack (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/FrameController;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V
public static synthetic fun Backstack$default (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/BackstackTransition;Landroidx/compose/animation/core/AnimationSpec;Lkotlin/jvm/functions/Function3;Lkotlin/jvm/functions/Function0;Ljava/lang/Object;Lkotlin/jvm/functions/Function3;ILjava/lang/Object;)V
public static final fun Backstack (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/BackstackTransition;Landroidx/compose/runtime/Composer;II)V
public static final fun Backstack (Ljava/util/List;Landroidx/compose/ui/Modifier;Lcom/zachklipp/compose/backstack/FrameController;Landroidx/compose/runtime/Composer;II)V
}

public abstract interface class com/zachklipp/compose/backstack/BackstackTransition {
Expand Down Expand Up @@ -30,15 +33,15 @@ public abstract interface class com/zachklipp/compose/backstack/FrameController
public abstract fun updateBackstack (Ljava/util/List;)V
}

public final class com/zachklipp/compose/backstack/FrameController$BackstackFrame {
public fun <init> (Ljava/lang/Object;Landroidx/compose/ui/Modifier;)V
public synthetic fun <init> (Ljava/lang/Object;Landroidx/compose/ui/Modifier;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Ljava/lang/Object;
public final class com/zachklipp/compose/backstack/FrameController$FrameAndModifier {
public fun <init> (Lcom/zachklipp/compose/backstack/BackstackFrame;Landroidx/compose/ui/Modifier;)V
public synthetic fun <init> (Lcom/zachklipp/compose/backstack/BackstackFrame;Landroidx/compose/ui/Modifier;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Lcom/zachklipp/compose/backstack/BackstackFrame;
public final fun component2 ()Landroidx/compose/ui/Modifier;
public final fun copy (Ljava/lang/Object;Landroidx/compose/ui/Modifier;)Lcom/zachklipp/compose/backstack/FrameController$BackstackFrame;
public static synthetic fun copy$default (Lcom/zachklipp/compose/backstack/FrameController$BackstackFrame;Ljava/lang/Object;Landroidx/compose/ui/Modifier;ILjava/lang/Object;)Lcom/zachklipp/compose/backstack/FrameController$BackstackFrame;
public final fun copy (Lcom/zachklipp/compose/backstack/BackstackFrame;Landroidx/compose/ui/Modifier;)Lcom/zachklipp/compose/backstack/FrameController$FrameAndModifier;
public static synthetic fun copy$default (Lcom/zachklipp/compose/backstack/FrameController$FrameAndModifier;Lcom/zachklipp/compose/backstack/BackstackFrame;Landroidx/compose/ui/Modifier;ILjava/lang/Object;)Lcom/zachklipp/compose/backstack/FrameController$FrameAndModifier;
public fun equals (Ljava/lang/Object;)Z
public final fun getKey ()Ljava/lang/Object;
public final fun getFrame ()Lcom/zachklipp/compose/backstack/BackstackFrame;
public final fun getModifier ()Landroidx/compose/ui/Modifier;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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")
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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"

Expand Down
Loading

0 comments on commit 8b55940

Please sign in to comment.