Introduces DialogCollator

Android notoriously give us no control over the z order of Dialog windows
other than taking care to `show()` them in the right order, so keeping a pile
of windows in sync with a list of `Overlay` rendering models is hard.

We've already solved that for individual instances of `LayeredDialogSessions`
(the delegate that does all of the work for `BodyAndOverlaysContainer`), but
it was still a problem when you nested them, e.g. in the style of the
recently introduced Nested Overlays sample. Also, the solution was kind of
nasty because we would replace any out of order Dialogs with brand new ones,
rather than preserving them by calling `dismiss()` and `show()`.

To fix all that we introduce `internal class DialogCollator`. On each view
update pass, a new `DialogCollator` instance is created by the outermost
`LayeredDialogSessions` and made available to any nested instances via the
`ViewEnvironment`. The shared collator collects a list of all existing
`DialogSession` instances, and a set of `DialogSessionUpdate` functions to be
asynchronously used to update existing instances or create new ones.

Because a `DialogCollator` has complete knowledge of all the windows managed
by the set of nested `LayerDialogSessions` instances it supports, it is able to
ensure that inserts are supported by calling `dismiss()` / `show()` on existing
windows in the sequence needed to reorder them.

One important change to note: our `SavedStateRegistry` code requires that each
window in a set has a unique name, which we derive by applying `Compatible.keyFor` to
its defining `Overlay`. We used to append to that key an `Overlay`'s position in its
`LayeredDialogSessions` list, as a poorly considered hack to simplify showing
several of the same type at once. That approach conflicts with reordering.

With this commit we no longer include the index value in `SavedStateRegistry` keys,
meaning that each `Overlay` shown by a `LayeredDialogSession` must have a unique
key -- `BackStackScreen` has long had a similar requirement. `NamedScreen`
exists to simplify that kind of thing, and in particular can be used to wrap
`ScreenOverlay.content` to meet the new requirement.

Fixes #966

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/views/MayBeLoadingScreen.kt

@@ -4,11 +4,11 @@ import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
 import com.squareup.sample.container.panel.ScrimScreen
 import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
 import com.squareup.workflow1.ui.container.BodyAndOverlaysScreen
-import com.squareup.workflow1.ui.container.FullScreenOverlay
+import com.squareup.workflow1.ui.container.FullScreenModal
 
 @OptIn(WorkflowUiExperimentalApi::class)
 typealias MayBeLoadingScreen =
-  BodyAndOverlaysScreen<ScrimScreen<OverviewDetailScreen>, FullScreenOverlay<LoaderSpinner>>
+  BodyAndOverlaysScreen<ScrimScreen<OverviewDetailScreen>, FullScreenModal<LoaderSpinner>>
 
 @OptIn(WorkflowUiExperimentalApi::class)
 fun MayBeLoadingScreen(
@@ -17,6 +17,6 @@ fun MayBeLoadingScreen(
 ): MayBeLoadingScreen {
   return BodyAndOverlaysScreen(
     ScrimScreen(baseScreen, dimmed = loaders.isNotEmpty()),
-    loaders.map { FullScreenOverlay(it) }
+    loaders.map { FullScreenModal(it) }
   )
 }

samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt

@@ -3,9 +3,12 @@ package com.squareup.sample.nestedoverlays
 import androidx.test.espresso.Espresso.onView
 import androidx.test.espresso.ViewInteraction
 import androidx.test.espresso.action.ViewActions.click
+import androidx.test.espresso.action.ViewActions.typeText
 import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
 import androidx.test.espresso.assertion.ViewAssertions.matches
+import androidx.test.espresso.matcher.RootMatchers.isDialog
 import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
+import androidx.test.espresso.matcher.ViewMatchers.withId
 import androidx.test.espresso.matcher.ViewMatchers.withParent
 import androidx.test.espresso.matcher.ViewMatchers.withParentIndex
 import androidx.test.espresso.matcher.ViewMatchers.withText
@@ -36,17 +39,17 @@ class NestedOverlaysAppTest {
     onBottomCoverEverything().assertDisplayed()
 
     onTopCoverBody().perform(click())
-    onView(withText("Close")).perform(click())
+    onView(withText("❌")).perform(click())
     onTopCoverEverything().perform(click())
-    onView(withText("Close")).perform(click())
+    onView(withText("❌")).perform(click())
 
-    onView(withText("Hide Top Bar")).perform(click())
+    onView(withText("Hide Top")).perform(click())
     onTopCoverBody().assertNotDisplayed()
     onTopCoverEverything().assertNotDisplayed()
     onBottomCoverBody().assertDisplayed()
     onBottomCoverEverything().assertDisplayed()
 
-    onView(withText("Hide Bottom Bar")).perform(click())
+    onView(withText("Hide Bottom")).perform(click())
     onTopCoverBody().assertNotDisplayed()
     onTopCoverEverything().assertNotDisplayed()
     onBottomCoverBody().assertNotDisplayed()
@@ -56,25 +59,109 @@ class NestedOverlaysAppTest {
   // https://github.com/square/workflow-kotlin/issues/966
   @Test fun canInsertDialog() {
     onTopCoverEverything().perform(click())
-    onView(withText("Hide Top Bar")).check(doesNotExist())
-    onView(withText("Cover Body")).perform(click())
 
-    // This line fails due to https://github.com/square/workflow-kotlin/issues/966
-    // onView(withText("Hide Top Bar")).check(doesNotExist())
+    // Cannot see the inner dialog.
+    onView(withText("Hide Top")).inRoot(isDialog()).check(doesNotExist())
+
+    // Click the outer dialog's button to show the inner dialog.
+    onView(withText("Cover Body")).inRoot(isDialog()).perform(click())
+    // Inner was created below outer, so we still can't see it.
+    onView(withText("Hide Top")).inRoot(isDialog()).check(doesNotExist())
+
+    // Close the outer dialog.
+    onView(withText("❌")).inRoot(isDialog()).perform(click())
+    // Now we can see the inner.
+    onView(withText("Hide Top")).inRoot(isDialog()).check(matches(isDisplayed()))
+    // Close it to confirm it really works.
+    onView(withText("❌")).inRoot(isDialog()).perform(click())
+    onTopCoverEverything().check(matches(isDisplayed()))
+  }
+
+  @Test fun canInsertAndRemoveCoveredDialog() {
+    // Show the outer dialog
+    onTopCoverEverything().perform(click())
+    // Show the inner dialog behind it
+    onView(withText("Cover Body")).inRoot(isDialog()).perform(click())
+    // Close the (covered) inner dialog and don't crash. :/
+    onView(withText("Reveal Body")).inRoot(isDialog()).perform(click())
+    // Close the outer dialog
+    onView(withText("❌")).inRoot(isDialog()).perform(click())
+    // We can see the activity window again
+    onTopCoverEverything().check(matches(isDisplayed()))
+  }
+
+  @Test fun whenReorderingViewStateIsPreserved() {
+    // Show the outer dialog
+    onTopCoverEverything().perform(click())
+
+    // Type something on it
+    onView(withId(R.id.button_bar_text)).inRoot(isDialog())
+      .perform(typeText("banana"))
+
+    // Click the outer dialog's button to show the inner dialog.
+    onView(withText("Cover Body")).inRoot(isDialog()).perform(click())
+
+    // The original outer dialog was destroyed and replaced.
+    // Check that the text we entered made it to the replacement dialog via view state.
+    onView(withId(R.id.button_bar_text)).inRoot(isDialog())
+      .check(matches(withText("banana")))
+  }
+
+  // https://github.com/square/workflow-kotlin/issues/314
+  @Test fun whenBodyAndOverlaysStopsBeingRenderedDialogsAreDismissed() {
+    onBottomCoverBody().perform(click())
+    onView(withText("💣")).inRoot(isDialog()).perform(click())
+
+    onBottomCoverBody().check(doesNotExist())
+    onView(withText("Reset")).perform(click())
 
-    // Should continue to close the top sheet and assert that the inner sheet is visible.
+    onBottomCoverBody().perform(click())
+    onView(withText("💣")).inRoot(isDialog()).check(matches(isDisplayed()))
   }
 
-  // So far can't express this in Espresso. Considering move to Maestro
-  // @Test fun canClickPastInnerWindow() {
-  //   onView(allOf(withText("Cover Everything"), withParent(withParentIndex(0))))
+  // So far can't express this in Espresso, because it refuses to work
+  // w/a root that lacks window focus. Considering move to Maestro.
+  // In the meantime I'd like to keep this commented out block around
+  // as a reminder.
+
+  // @Test fun canCoverDialogAndRemoveItWhileCovered() {
+  //   // Show the inner dialog
+  //   onTopCoverBody().perform(click())
+  //
+  //   lateinit var activity: Activity
+  //   scenarioRule.scenario.onActivity { activity = it }
+  //
+  //   // Show the outer dialog
+  //   onTopCoverEverything()
+  //     .inRoot(
+  //       allOf(
+  //         withDecorView(Matchers.`is`(activity.window.decorView)),
+  //         Matchers.not(hasWindowFocus())
+  //       )
+  //     )
   //     .perform(click())
   //
-  //   scenario.onActivity { activity ->
-  //     onView(allOf(withText("Cover Everything"), withParent(withParentIndex(0))))
-  //       .inRoot(withDecorView(not(`is`(activity.window.decorView))))
-  //       .perform(click())
-  //   }
+  //   // Close the (covered) inner dialog
+  //   onView(withText("Reveal Body")).inRoot(isDialog()).perform(click())
+  //   // Close the outer dialog
+  //   onView(withText("❌")).inRoot(isDialog()).perform(click())
+  //   // We can see the activity window again
+  //   onTopCoverEverything().check(matches(isDisplayed()))
+  // }
+  //
+  // /**
+  //  * Like the private (why?) `hasWindowFocus` method in Espresso, but
+  //  * built into a `Matcher<Root>` rather than a `Matcher<View>` (since
+  //  * that was our only use case).
+  //  */
+  // fun hasWindowFocus(): Matcher<Root> {
+  //   return withDecorView(object : TypeSafeMatcher<View>() {
+  //     override fun describeTo(description: Description) {
+  //       description.appendText("has window focus (Square fork)")
+  //     }
+  //
+  //     override fun matchesSafely(item: View): Boolean = item.hasWindowFocus()
+  //   })
   // }
 
   private fun ViewInteraction.assertNotDisplayed() {

samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/ButtonBar.kt

@@ -2,6 +2,9 @@ package com.squareup.sample.nestedoverlays
 
 import android.graphics.drawable.ColorDrawable
 import android.view.Gravity
+import android.view.View.GONE
+import android.view.View.VISIBLE
+import android.widget.EditText
 import android.widget.LinearLayout
 import androidx.annotation.ColorRes
 import androidx.annotation.StringRes
@@ -21,23 +24,33 @@ data class Button(
 class ButtonBar(
   vararg buttons: Button?,
   @ColorRes val color: Int = -1,
+  val showEditText: Boolean = false,
 ) : AndroidScreen<ButtonBar> {
   private val buttons: List<Button> = buttons.filterNotNull().toList()
 
   override val viewFactory =
     ScreenViewFactory.fromCode<ButtonBar> { _, initialEnvironment, context, _ ->
       LinearLayout(context).let { view ->
-        @Suppress("DEPRECATION")
-        if (color > -1) view.background = ColorDrawable(view.resources.getColor(color))
+        // Child 0 is always an EditText, which may or may not be visible.
+        val editText = EditText(context)
+        editText.id = R.id.button_bar_text
+        view.addView(editText)
 
         view.gravity = Gravity.CENTER
 
-        ScreenViewHolder(initialEnvironment, view) { bar, _ ->
-          val existing = view.childCount
+        ScreenViewHolder(initialEnvironment, view) { newBar, _ ->
+          @Suppress("DEPRECATION")
+          view.background =
+            if (newBar.color > -1) ColorDrawable(view.resources.getColor(newBar.color)) else null
 
-          bar.buttons.forEachIndexed { index, button ->
-            val buttonView = if (index < existing) {
-              view[index] as ButtonView
+          editText.visibility = if (newBar.showEditText) VISIBLE else GONE
+
+          // After the EditText, an arbitrary number of ButtonView.
+          val existingButtonCount = view.childCount - 1
+
+          newBar.buttons.forEachIndexed { index, button ->
+            val buttonView = if (index < existingButtonCount) {
+              view[index + 1] as ButtonView
             } else {
               ButtonView(context).also { view.addView(it) }
             }
@@ -46,7 +59,7 @@ class ButtonBar(
               setOnClickListener { button.onClick() }
             }
           }
-          for (i in bar.buttons.size until view.childCount) view.removeViewAt(i)
+          for (i in newBar.buttons.size + 1 until view.childCount) view.removeViewAt(i)
         }
       }
     }

samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/NestedOverlaysWorkflow.kt

@@ -8,20 +8,15 @@ import com.squareup.workflow1.StatefulWorkflow
 import com.squareup.workflow1.ui.Screen
 import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
 import com.squareup.workflow1.ui.container.BodyAndOverlaysScreen
-import com.squareup.workflow1.ui.container.FullScreenOverlay
-import com.squareup.workflow1.ui.container.Overlay
+import com.squareup.workflow1.ui.container.FullScreenModal
 
-typealias NestedOverlaysRendering = BodyAndOverlaysScreen<
-  TopAndBottomBarsScreen<BodyAndOverlaysScreen<Screen, Overlay>>,
-  Overlay
-  >
-
-object NestedOverlaysWorkflow : StatefulWorkflow<Unit, State, Nothing, NestedOverlaysRendering>() {
+object NestedOverlaysWorkflow : StatefulWorkflow<Unit, State, Nothing, Screen>() {
   data class State(
     val showTopBar: Boolean = true,
     val showBottomBar: Boolean = true,
     val showInnerSheet: Boolean = false,
-    val showOuterSheet: Boolean = false
+    val showOuterSheet: Boolean = false,
+    val nuked: Boolean = false
   )
 
   override fun initialState(
@@ -33,44 +28,54 @@ object NestedOverlaysWorkflow : StatefulWorkflow<Unit, State, Nothing, NestedOve
     renderProps: Unit,
     renderState: State,
     context: RenderContext
-  ): NestedOverlaysRendering {
+  ): Screen {
+    if (renderState.nuked) {
+      return ButtonBar(Button(R.string.reset, context.eventHandler { state = State() }))
+    }
+
     val toggleTopBarButton = Button(
-      name = if (renderState.showTopBar) R.string.HIDE_TOP else R.string.SHOW_TOP,
+      name = if (renderState.showTopBar) R.string.hide_top else R.string.show_top,
       onClick = context.eventHandler { state = state.copy(showTopBar = !state.showTopBar) }
     )
 
     val toggleBottomBarButton = Button(
-      name = if (renderState.showBottomBar) R.string.HIDE_BOTTOM else R.string.SHOW_BOTTOM,
+      name = if (renderState.showBottomBar) R.string.hide_bottom else R.string.show_bottom,
       onClick = context.eventHandler { state = state.copy(showBottomBar = !state.showBottomBar) }
     )
 
     val outerSheet = if (!renderState.showOuterSheet) {
       null
     } else {
-      FullScreenOverlay(
+      FullScreenModal(
         ButtonBar(
           Button(
-            name = R.string.CLOSE,
+            name = R.string.close,
             onClick = context.eventHandler { state = state.copy(showOuterSheet = false) }
           ),
           context.toggleInnerSheetButton(renderState),
-          color = android.R.color.holo_green_light
+          color = android.R.color.holo_green_light,
+          showEditText = true
         )
       )
     }
 
     val innerSheet = if (!renderState.showInnerSheet) {
       null
     } else {
-      FullScreenOverlay(
+      FullScreenModal(
         ButtonBar(
           Button(
-            name = R.string.CLOSE,
+            name = R.string.close,
             onClick = context.eventHandler { state = state.copy(showInnerSheet = false) }
           ),
           toggleTopBarButton,
           toggleBottomBarButton,
-          color = android.R.color.holo_red_light
+          Button(
+            name = R.string.nuke,
+            onClick = context.eventHandler { state = State(nuked = true) }
+          ),
+          color = android.R.color.holo_red_light,
+          showEditText = true
         )
       )
     }
@@ -98,14 +103,14 @@ object NestedOverlaysWorkflow : StatefulWorkflow<Unit, State, Nothing, NestedOve
   ) = ButtonBar(
     toggleInnerSheetButton(renderState),
     Button(
-      name = R.string.COVER_ALL,
+      name = R.string.cover_all,
       onClick = eventHandler { state = state.copy(showOuterSheet = true) }
     )
   )
 
   private fun RenderContext.toggleInnerSheetButton(renderState: State) =
     Button(
-      name = if (renderState.showInnerSheet) R.string.REVEAL_BODY else R.string.COVER_BODY,
+      name = if (renderState.showInnerSheet) R.string.reveal_body else R.string.cover_body,
       onClick = eventHandler {
         state = state.copy(showInnerSheet = !state.showInnerSheet)
       }

samples/nested-overlays/src/main/res/values/ids.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+  <item name="button_bar_text" type="id"/>
+</resources>

samples/nested-overlays/src/main/res/values/strings.xml

@@ -1,13 +1,15 @@
 <resources>
+  <string name="body">body</string>
+  <string name="bottom">bottom</string>
+  <string name="close">❌</string>
+  <string name="cover_all">Cover Everything</string>
+  <string name="cover_body">Cover Body</string>
+  <string name="hide_bottom">Hide Bottom</string>
+  <string name="hide_top">Hide Top</string>
+  <string name="nuke">💣</string>
+  <string name="reset">Reset</string>
+  <string name="reveal_body">Reveal Body</string>
+  <string name="show_bottom">Show Bottom Bar</string>
+  <string name="show_top">Show Top Bar</string>
   <string name="app_name">Nested Overlays</string>
-  <string name="BODY">body</string>
-  <string name="COVER_BODY">Cover Body</string>
-  <string name="BOTTOM">bottom</string>
-  <string name="HIDE_BOTTOM">Hide Bottom Bar</string>
-  <string name="SHOW_BOTTOM">Show Bottom Bar</string>
-  <string name="HIDE_TOP">Hide Top Bar</string>
-  <string name="SHOW_TOP">Show Top Bar</string>
-  <string name="REVEAL_BODY">Reveal Body</string>
-  <string name="COVER_ALL">Cover Everything</string>
-  <string name="CLOSE">Close</string>
 </resources>