Skip to content

Commit 5eb88ab

Browse files
rjrjrrjrjr
authored
Merge pull request #656 from square/ray/nested-stateregistry
ModalContainer provides SavedStateRegistries to each of its layers.
2 parents 12d54f8 + 3633994 commit 5eb88ab

File tree

20 files changed

+776
-701
lines changed

20 files changed

+776
-701
lines changed

‎samples/containers/android/src/main/java/com/squareup/sample/container/overviewdetail/OverviewDetailContainer.kt

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Detail
88
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Overview
99
import com.squareup.sample.container.overviewdetail.OverviewDetailConfig.Single
1010
import com.squareup.workflow1.ui.LayoutRunner
11+
import com.squareup.workflow1.ui.Named
1112
import com.squareup.workflow1.ui.ViewEnvironment
1213
import com.squareup.workflow1.ui.ViewFactory
1314
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
1415
import com.squareup.workflow1.ui.WorkflowViewStub
1516
import com.squareup.workflow1.ui.backstack.BackStackScreen
16-
import com.squareup.workflow1.ui.backstack.withBackStackStateKeyPrefix
1717

1818
/**
1919
* Displays [OverviewDetailScreen] renderings in either split pane or single pane
@@ -53,11 +53,13 @@ class OverviewDetailContainer(view: View) : LayoutRunner<OverviewDetailScreen> {
5353
if (rendering.detailRendering == null && rendering.selectDefault != null) {
5454
rendering.selectDefault!!.invoke()
5555
} else {
56-
// Since we have two sibling backstacks, we need to give them each different
57-
// SavedStateRegistry key prefixes.
58-
val overviewViewEnvironment = viewEnvironment
59-
.withBackStackStateKeyPrefix(OverviewBackStackKey) + (OverviewDetailConfig to Overview)
60-
overviewStub!!.update(rendering.overviewRendering, overviewViewEnvironment)
56+
val overviewViewEnvironment = viewEnvironment + (OverviewDetailConfig to Overview)
57+
58+
// Without this name, the two BackStackScreen containers will try
59+
// to sign up with SavedStateRegistry with the same id, and crash.
60+
val overviewRendering = Named(rendering.overviewRendering, "Overview")
61+
overviewStub!!.update(overviewRendering, overviewViewEnvironment)
62+
6163
rendering.detailRendering
6264
?.let { detail ->
6365
detailStub!!.actual.visibility = VISIBLE
@@ -87,8 +89,5 @@ class OverviewDetailContainer(view: View) : LayoutRunner<OverviewDetailScreen> {
8789
companion object : ViewFactory<OverviewDetailScreen> by LayoutRunner.bind(
8890
layoutId = R.layout.overview_detail,
8991
constructor = ::OverviewDetailContainer
90-
) {
91-
private const val OverviewBackStackKey = "overview"
92-
private const val DetailBackStackKey = "detail"
93-
}
92+
)
9493
}

‎samples/tictactoe/app/src/androidTest/java/com/squareup/sample/TicTacToeEspressoTest.kt

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import androidx.test.espresso.action.ViewActions.typeText
1111
import androidx.test.espresso.assertion.ViewAssertions.matches
1212
import androidx.test.espresso.matcher.ViewMatchers.hasDescendant
1313
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
14+
import androidx.test.espresso.matcher.ViewMatchers.withClassName
1415
import androidx.test.espresso.matcher.ViewMatchers.withId
16+
import androidx.test.espresso.matcher.ViewMatchers.withParent
17+
import androidx.test.espresso.matcher.ViewMatchers.withParentIndex
1518
import androidx.test.espresso.matcher.ViewMatchers.withText
1619
import androidx.test.ext.junit.rules.ActivityScenarioRule
1720
import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -29,6 +32,8 @@ import com.squareup.workflow1.ui.internal.test.DetectLeaksAfterTestSuccess
2932
import com.squareup.workflow1.ui.internal.test.IdlingDispatcherRule
3033
import com.squareup.workflow1.ui.internal.test.actuallyPressBack
3134
import com.squareup.workflow1.ui.internal.test.inAnyView
35+
import org.hamcrest.CoreMatchers.allOf
36+
import org.hamcrest.CoreMatchers.endsWith
3237
import org.junit.After
3338
import org.junit.Before
3439
import org.junit.Rule
@@ -190,6 +195,38 @@ class TicTacToeEspressoTest {
190195
inAnyView(withId(R.id.login_email)).check(matches(withText("foo@2fa")))
191196
}
192197

198+
/**
199+
* On tablets this revealed a problem with SavedStateRegistry.
200+
* https://github.com/square/workflow-kotlin/pull/656#issuecomment-1027274391
201+
*/
202+
@Test fun fullJourney() {
203+
inAnyView(withId(R.id.login_email)).type("foo@bar")
204+
inAnyView(withId(R.id.login_password)).type("password")
205+
inAnyView(withId(R.id.login_button)).perform(click())
206+
207+
inAnyView(withId(R.id.start_game)).perform(click())
208+
209+
clickCell(0)
210+
clickCell(3)
211+
clickCell(1)
212+
clickCell(4)
213+
clickCell(2)
214+
215+
inAnyView(withText(R.string.exit)).perform(click())
216+
inAnyView(withId(R.id.start_game)).check(matches(isDisplayed()))
217+
actuallyPressBack()
218+
inAnyView(withId(R.id.login_email)).check(matches(isDisplayed()))
219+
}
220+
221+
private fun clickCell(index: Int) {
222+
inAnyView(
223+
allOf(
224+
withParent(withClassName(endsWith("GridLayout"))),
225+
withParentIndex(index)
226+
)
227+
).perform((click()))
228+
}
229+
193230
private fun ViewInteraction.type(text: String) {
194231
perform(typeText(text), closeSoftKeyboard())
195232
}

‎workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/ComposeViewTreeIntegrationTest.kt

Lines changed: 130 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import androidx.compose.ui.test.performClick
2626
import com.google.common.truth.Truth.assertThat
2727
import com.squareup.workflow1.ui.AndroidViewRendering
2828
import com.squareup.workflow1.ui.Compatible
29-
import com.squareup.workflow1.ui.Named
3029
import com.squareup.workflow1.ui.ViewEnvironment
3130
import com.squareup.workflow1.ui.ViewFactory
3231
import com.squareup.workflow1.ui.ViewRegistry
@@ -394,7 +393,7 @@ internal class ComposeViewTreeIntegrationTest {
394393
}
395394

396395
@Test fun composition_is_restored_in_multiple_modals_after_config_change() {
397-
val firstScreen = ComposeRendering(compatibilityKey = "first") {
396+
val firstScreen = ComposeRendering(compatibilityKey = "key") {
398397
var counter by rememberSaveable { mutableStateOf(0) }
399398
BasicText(
400399
"Counter: $counter",
@@ -403,7 +402,9 @@ internal class ComposeViewTreeIntegrationTest {
403402
.testTag(CounterTag)
404403
)
405404
}
406-
val secondScreen = ComposeRendering(compatibilityKey = "second") {
405+
// Use the same compatibility key – these screens are in different modals, so they won't
406+
// conflict.
407+
val secondScreen = ComposeRendering(compatibilityKey = "key") {
407408
var counter by rememberSaveable { mutableStateOf(0) }
408409
BasicText(
409410
"Counter2: $counter",
@@ -412,17 +413,26 @@ internal class ComposeViewTreeIntegrationTest {
412413
.testTag(CounterTag2)
413414
)
414415
}
416+
// Use the same compatibility key – these screens are in different modals, so they won't
417+
// conflict.
418+
val thirdScreen = ComposeRendering(compatibilityKey = "key") {
419+
var counter by rememberSaveable { mutableStateOf(0) }
420+
BasicText(
421+
"Counter3: $counter",
422+
Modifier
423+
.clickable { counter++ }
424+
.testTag(CounterTag3)
425+
)
426+
}
415427

416428
// Show first screen to initialize state.
417429
scenario.onActivity {
418430
it.setRendering(
419431
TestModalScreen(
420432
listOf(
421-
// Name each BackStackScreen to give them unique state registry keys.
422-
// TODO(https://github.com/square/workflow-kotlin/issues/469) Should this naming be
423-
// done automatically in ModalContainer?
424-
Named(BackStackScreen(EmptyRendering, firstScreen), "modal1"),
425-
Named(BackStackScreen(EmptyRendering, secondScreen), "modal2")
433+
firstScreen,
434+
secondScreen,
435+
thirdScreen
426436
)
427437
)
428438
)
@@ -438,13 +448,124 @@ internal class ComposeViewTreeIntegrationTest {
438448
.performClick()
439449
.assertTextEquals("Counter2: 1")
440450

451+
composeRule.onNodeWithTag(CounterTag3)
452+
.assertTextEquals("Counter3: 0")
453+
.performClick()
454+
.assertTextEquals("Counter3: 1")
455+
441456
scenario.recreate()
442457

443458
composeRule.onNodeWithTag(CounterTag)
444459
.assertTextEquals("Counter: 1")
445460

446461
composeRule.onNodeWithTag(CounterTag2)
447462
.assertTextEquals("Counter2: 1")
463+
464+
composeRule.onNodeWithTag(CounterTag3)
465+
.assertTextEquals("Counter3: 1")
466+
}
467+
468+
@Test fun composition_is_restored_in_multiple_modals_backstacks_after_config_change() {
469+
fun createRendering(
470+
layer: Int,
471+
screen: Int
472+
) = ComposeRendering(
473+
// Use the same compatibility key across layers – these screens are in different modals, so
474+
// they won't conflict.
475+
compatibilityKey = screen.toString()
476+
) {
477+
var counter by rememberSaveable { mutableStateOf(0) }
478+
BasicText(
479+
"Counter[$layer][$screen]: $counter",
480+
Modifier
481+
.clickable { counter++ }
482+
.testTag("L${layer}S$screen")
483+
)
484+
}
485+
486+
val layer0Screen0 = createRendering(0, 0)
487+
val layer0Screen1 = createRendering(0, 1)
488+
val layer1Screen0 = createRendering(1, 0)
489+
val layer1Screen1 = createRendering(1, 1)
490+
491+
// Show first screen to initialize state.
492+
scenario.onActivity {
493+
it.setRendering(
494+
TestModalScreen(
495+
listOf(
496+
BackStackScreen(EmptyRendering, layer0Screen0),
497+
BackStackScreen(EmptyRendering, layer1Screen0)
498+
)
499+
)
500+
)
501+
}
502+
503+
composeRule.onNodeWithTag("L0S0")
504+
.assertTextEquals("Counter[0][0]: 0")
505+
.assertIsDisplayed()
506+
.performClick()
507+
.assertTextEquals("Counter[0][0]: 1")
508+
509+
composeRule.onNodeWithTag("L1S0")
510+
.assertTextEquals("Counter[1][0]: 0")
511+
.assertIsDisplayed()
512+
.performClick()
513+
.assertTextEquals("Counter[1][0]: 1")
514+
515+
// Push some screens onto the backstack.
516+
scenario.onActivity {
517+
it.setRendering(
518+
TestModalScreen(
519+
listOf(
520+
BackStackScreen(EmptyRendering, layer0Screen0, layer0Screen1),
521+
BackStackScreen(EmptyRendering, layer1Screen0, layer1Screen1)
522+
)
523+
)
524+
)
525+
}
526+
527+
composeRule.onNodeWithTag("L0S0")
528+
.assertDoesNotExist()
529+
composeRule.onNodeWithTag("L1S0")
530+
.assertDoesNotExist()
531+
532+
composeRule.onNodeWithTag("L0S1")
533+
.assertTextEquals("Counter[0][1]: 0")
534+
.assertIsDisplayed()
535+
.performClick()
536+
.assertTextEquals("Counter[0][1]: 1")
537+
538+
composeRule.onNodeWithTag("L1S1")
539+
.assertTextEquals("Counter[1][1]: 0")
540+
.assertIsDisplayed()
541+
.performClick()
542+
.assertTextEquals("Counter[1][1]: 1")
543+
544+
// Simulate config change.
545+
scenario.recreate()
546+
547+
// Check that the last-shown screens were restored.
548+
composeRule.onNodeWithTag("L0S1")
549+
.assertIsDisplayed()
550+
composeRule.onNodeWithTag("L1S1")
551+
.assertIsDisplayed()
552+
553+
// Pop both backstacks and check that screens were restored.
554+
scenario.onActivity {
555+
it.setRendering(
556+
TestModalScreen(
557+
listOf(
558+
BackStackScreen(EmptyRendering, layer0Screen0),
559+
BackStackScreen(EmptyRendering, layer1Screen0)
560+
)
561+
)
562+
)
563+
}
564+
565+
composeRule.onNodeWithText("Counter[0][0]: 1")
566+
.assertIsDisplayed()
567+
composeRule.onNodeWithText("Counter[1][0]: 1")
568+
.assertIsDisplayed()
448569
}
449570

450571
private fun WorkflowUiTestActivity.setBackstack(vararg backstack: ComposeRendering) {
@@ -495,5 +616,6 @@ internal class ComposeViewTreeIntegrationTest {
495616

496617
const val CounterTag = "counter"
497618
const val CounterTag2 = "counter2"
619+
const val CounterTag3 = "counter3"
498620
}
499621
}

‎workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/RenderAsStateTest.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import kotlinx.coroutines.job
3636
import kotlinx.coroutines.test.TestCoroutineScope
3737
import okio.ByteString
3838
import okio.ByteString.Companion.decodeBase64
39-
import org.junit.Ignore
4039
import org.junit.Rule
4140
import org.junit.Test
4241
import org.junit.rules.RuleChain
@@ -255,7 +254,6 @@ internal class RenderAsStateTest {
255254
}
256255
}
257256

258-
@Ignore("https://github.com/square/workflow-kotlin/issues/504")
259257
@Test fun runtimeIsCancelledWhenCompositionFails() {
260258
var innerJob: Job? = null
261259
val workflow = Workflow.stateless<Unit, Nothing, Unit> {

‎workflow-ui/container-android/api/container-android.api

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -33,38 +33,11 @@ public final class com/squareup/workflow1/ui/backstack/BackStackContainer$Compan
3333
public fun getType ()Lkotlin/reflect/KClass;
3434
}
3535

36-
public final class com/squareup/workflow1/ui/backstack/BackStackStateKeyKt {
37-
public static final fun getGetBackStackStateKeyPrefix (Lcom/squareup/workflow1/ui/ViewEnvironment;)Ljava/lang/String;
38-
public static final fun withBackStackStateKeyPrefix (Lcom/squareup/workflow1/ui/ViewEnvironment;Ljava/lang/String;)Lcom/squareup/workflow1/ui/ViewEnvironment;
39-
}
40-
41-
public final class com/squareup/workflow1/ui/backstack/KeyedStateRegistryOwner : androidx/lifecycle/LifecycleOwner, androidx/savedstate/SavedStateRegistryOwner {
42-
public static final field Companion Lcom/squareup/workflow1/ui/backstack/KeyedStateRegistryOwner$Companion;
43-
public synthetic fun <init> (Ljava/lang/String;Landroidx/lifecycle/LifecycleOwner;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
44-
public final fun getController ()Landroidx/savedstate/SavedStateRegistryController;
45-
public final fun getKey ()Ljava/lang/String;
46-
public fun getLifecycle ()Landroidx/lifecycle/Lifecycle;
47-
public fun getSavedStateRegistry ()Landroidx/savedstate/SavedStateRegistry;
48-
}
49-
50-
public final class com/squareup/workflow1/ui/backstack/KeyedStateRegistryOwner$Companion {
51-
public final fun installAsSavedStateRegistryOwnerOn (Landroid/view/View;Ljava/lang/String;)Lcom/squareup/workflow1/ui/backstack/KeyedStateRegistryOwner;
52-
}
53-
54-
public final class com/squareup/workflow1/ui/backstack/StateRegistryAggregator {
55-
public fun <init> (Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;)V
56-
public final fun attachToParentRegistry (Ljava/lang/String;Landroidx/savedstate/SavedStateRegistryOwner;)V
57-
public final fun detachFromParentRegistry ()V
58-
public final fun pruneKeys (Ljava/util/Collection;)V
59-
public final fun restoreRegistryControllerIfReady (Ljava/lang/String;Landroidx/savedstate/SavedStateRegistryController;)V
60-
public final fun saveRegistryController (Ljava/lang/String;Landroidx/savedstate/SavedStateRegistryController;)V
61-
}
62-
6336
public final class com/squareup/workflow1/ui/backstack/ViewStateCache : android/os/Parcelable {
6437
public static final field CREATOR Lcom/squareup/workflow1/ui/backstack/ViewStateCache$CREATOR;
6538
public fun <init> ()V
6639
public fun <init> (Ljava/util/Map;)V
67-
public final fun attachToParentRegistry (Ljava/lang/String;Landroidx/savedstate/SavedStateRegistryOwner;)V
40+
public final fun attachToParentRegistryOwner (Ljava/lang/String;Landroidx/savedstate/SavedStateRegistryOwner;)V
6841
public fun describeContents ()I
6942
public final fun detachFromParentRegistry ()V
7043
public final fun getViewStates$wf1_container_android ()Ljava/util/Map;
@@ -148,31 +121,31 @@ public abstract class com/squareup/workflow1/ui/modal/ModalContainer : android/w
148121
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;II)V
149122
public synthetic fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;IIILkotlin/jvm/internal/DefaultConstructorMarker;)V
150123
protected abstract fun buildDialog (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)Lcom/squareup/workflow1/ui/modal/ModalContainer$DialogRef;
124+
protected fun onAttachedToWindow ()V
125+
protected fun onDetachedFromWindow ()V
151126
protected fun onRestoreInstanceState (Landroid/os/Parcelable;)V
152127
protected fun onSaveInstanceState ()Landroid/os/Parcelable;
153128
protected final fun update (Lcom/squareup/workflow1/ui/modal/HasModals;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
154129
protected abstract fun updateDialog (Lcom/squareup/workflow1/ui/modal/ModalContainer$DialogRef;)V
155130
}
156131

157132
protected final class com/squareup/workflow1/ui/modal/ModalContainer$DialogRef {
133+
public field savedStateRegistryKey Ljava/lang/String;
158134
public fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Ljava/lang/Object;)V
159135
public synthetic fun <init> (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Ljava/lang/Object;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
160-
public final fun component1 ()Ljava/lang/Object;
161-
public final fun component2 ()Lcom/squareup/workflow1/ui/ViewEnvironment;
162-
public final fun component3 ()Landroid/app/Dialog;
163-
public final fun component4 ()Ljava/lang/Object;
164136
public final fun copy (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Ljava/lang/Object;)Lcom/squareup/workflow1/ui/modal/ModalContainer$DialogRef;
165137
public static synthetic fun copy$default (Lcom/squareup/workflow1/ui/modal/ModalContainer$DialogRef;Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Ljava/lang/Object;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/modal/ModalContainer$DialogRef;
166138
public final fun dismiss$wf1_container_android ()V
167139
public fun equals (Ljava/lang/Object;)Z
168140
public final fun getDialog ()Landroid/app/Dialog;
169141
public final fun getExtra ()Ljava/lang/Object;
170142
public final fun getModalRendering ()Ljava/lang/Object;
143+
public final fun getSavedStateRegistryKey$wf1_container_android ()Ljava/lang/String;
171144
public final fun getViewEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
172145
public fun hashCode ()I
173146
public final fun restore$wf1_container_android (Lcom/squareup/workflow1/ui/modal/ModalContainer$KeyAndBundle;)V
174147
public final fun save$wf1_container_android ()Lcom/squareup/workflow1/ui/modal/ModalContainer$KeyAndBundle;
175-
public fun toString ()Ljava/lang/String;
148+
public final fun setSavedStateRegistryKey$wf1_container_android (Ljava/lang/String;)V
176149
}
177150

178151
public final class com/squareup/workflow1/ui/modal/ModalContainer$KeyAndBundle : android/os/Parcelable {

‎workflow-ui/container-android/src/androidTest/java/com/squareup/workflow1/ui/backstack/test/BackstackContainerTest.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ internal class BackstackContainerTest {
6868
}
6969
}
7070

71+
/**
72+
* Note that this test passes on older AVDs. It fails reliably on API 32 w/o the fix for
73+
* https://github.com/square/workflow-kotlin/issues/570
74+
*/
7175
@Test fun state_restores_after_recreate_without_crashing() {
7276
assertThat(scenario.state).isEqualTo(RESUMED)
7377

0 commit comments

Comments
 (0)