Introduces DialogCollator

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>