Fixes some Dialog handling problems revealed by Nested Overlays sample

- Replaces `View.getGlobalVisibleRect` calls with new, more robust `View.getScreenRect` extension

- `Dialog.setBounds` sets `FLAG_LAYOUT_IN_SCREEN` to ensure `Dialog` is really placed where we want it.
  Otherwise it may be offset by the height of the toolbar. I think we've missed this up until
  now because our other samples and prod use all have richer themes than the default codepaths
  set up.

- Default `ScreenOverlayDialogFactory` implementation disables dimming, for least confusing
  out of box behavior.

Note that these tweaks do not address #966, that's a bigger effort.

Author: rjrjr

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

@@ -76,7 +76,7 @@ class NestedOverlaysAppTest {
   //       .perform(click())
   //   }
   // }
-  
+
   private fun ViewInteraction.assertNotDisplayed() {
     check(matches(not(isDisplayed())))
   }

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

@@ -22,11 +22,12 @@ class ButtonBar(
   vararg buttons: Button?,
   @ColorRes val color: Int = -1,
 ) : AndroidScreen<ButtonBar> {
-  val buttons: List<Button> = buttons.filterNotNull().toList()
+  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))
 
         view.gravity = Gravity.CENTER

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

@@ -44,28 +44,36 @@ object NestedOverlaysWorkflow : StatefulWorkflow<Unit, State, Nothing, NestedOve
       onClick = context.eventHandler { state = state.copy(showBottomBar = !state.showBottomBar) }
     )
 
-    val outerSheet = if (!renderState.showOuterSheet) null else FullScreenOverlay(
-      ButtonBar(
-        Button(
-          name = R.string.CLOSE,
-          onClick = context.eventHandler { state = state.copy(showOuterSheet = false) }
-        ),
-        context.toggleInnerSheetButton(renderState),
-        color = android.R.color.holo_green_light
+    val outerSheet = if (!renderState.showOuterSheet) {
+      null
+    } else {
+      FullScreenOverlay(
+        ButtonBar(
+          Button(
+            name = R.string.CLOSE,
+            onClick = context.eventHandler { state = state.copy(showOuterSheet = false) }
+          ),
+          context.toggleInnerSheetButton(renderState),
+          color = android.R.color.holo_green_light
+        )
       )
-    )
+    }
 
-    val innerSheet = if (!renderState.showInnerSheet) null else FullScreenOverlay(
-      ButtonBar(
-        Button(
-          name = R.string.CLOSE,
-          onClick = context.eventHandler { state = state.copy(showInnerSheet = false) }
-        ),
-        toggleTopBarButton,
-        toggleBottomBarButton,
-        color = android.R.color.holo_red_light
+    val innerSheet = if (!renderState.showInnerSheet) {
+      null
+    } else {
+      FullScreenOverlay(
+        ButtonBar(
+          Button(
+            name = R.string.CLOSE,
+            onClick = context.eventHandler { state = state.copy(showInnerSheet = false) }
+          ),
+          toggleTopBarButton,
+          toggleBottomBarButton,
+          color = android.R.color.holo_red_light
+        )
       )
-    )
+    }
     val bodyBarButtons = ButtonBar(toggleTopBarButton, toggleBottomBarButton)
 
     return BodyAndOverlaysScreen(

workflow-ui/core-android/api/core-android.api

@@ -306,6 +306,7 @@ public final class com/squareup/workflow1/ui/container/AlertOverlayDialogFactory
 }
 
 public final class com/squareup/workflow1/ui/container/AndroidDialogBoundsKt {
+	public static final fun getScreenRect (Landroid/view/View;Landroid/graphics/Rect;)V
 	public static final fun setBounds (Landroid/app/Dialog;Landroid/graphics/Rect;)V
 }
 

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/AndroidDialogBounds.kt

@@ -1,10 +1,12 @@
 package com.squareup.workflow1.ui.container
 
+import android.annotation.SuppressLint
 import android.app.Dialog
 import android.graphics.Rect
 import android.view.Gravity
 import android.view.View
 import android.view.Window
+import android.view.WindowManager.LayoutParams.FLAG_LAYOUT_IN_SCREEN
 import com.squareup.workflow1.ui.ViewEnvironment
 import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
 import kotlinx.coroutines.CoroutineScope
@@ -16,16 +18,19 @@ import kotlinx.coroutines.flow.onEach
 
 /**
  * Updates the size of the [Window] of the receiving [Dialog].
- * [bounds] is expected to be in global display coordinates,
- * e.g. as returned from [View.getGlobalVisibleRect].
+ * [bounds] is expected to be in global screen coordinates,
+ * as returned from [View.getScreenRect].
  *
  * @see OverlayDialogHolder.onUpdateBounds
  */
 @WorkflowUiExperimentalApi
 public fun Dialog.setBounds(bounds: Rect) {
   window?.let {
+    it.addFlags(FLAG_LAYOUT_IN_SCREEN)
     it.attributes = it.attributes.apply {
-      gravity = Gravity.TOP or Gravity.START
+      // Our absolute coordinates are independent from Rtl.
+      @SuppressLint("RtlHardcoded")
+      gravity = Gravity.LEFT or Gravity.TOP // we use absolute coordinates.
       width = bounds.width()
       height = bounds.height()
       x = bounds.left
@@ -34,6 +39,23 @@ public fun Dialog.setBounds(bounds: Rect) {
   }
 }
 
+/**
+ * Returns the bounds of this [View] in the coordinate space of the device
+ * screen, based on [View.getLocationOnScreen] and its reported [width][View.getWidth]
+ * and [height][View.getHeight].
+ */
+@WorkflowUiExperimentalApi
+public fun View.getScreenRect(rect: Rect) {
+  val coordinates = IntArray(2)
+  getLocationOnScreen(coordinates)
+  rect.apply {
+    left = coordinates[0]
+    top = coordinates[1]
+    right = left + width
+    bottom = top + height
+  }
+}
+
 @WorkflowUiExperimentalApi
 internal fun <D : Dialog> D.maintainBounds(
   environment: ViewEnvironment,

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt

@@ -279,7 +279,7 @@ public class LayeredDialogSessions private constructor(
       superDispatchTouchEvent: (MotionEvent) -> Unit
     ): LayeredDialogSessions {
       val boundsRect = Rect()
-      if (view.isAttachedToWindow) view.getGlobalVisibleRect(boundsRect)
+      if (view.isAttachedToWindow) view.getScreenRect(boundsRect)
       val boundsStateFlow = MutableStateFlow(Rect(boundsRect))
 
       return LayeredDialogSessions(
@@ -326,9 +326,8 @@ public class LayeredDialogSessions private constructor(
         // accordingly.
         val attachStateChangeListener = object : OnAttachStateChangeListener {
           val boundsListener = OnGlobalLayoutListener {
-            if (view.getGlobalVisibleRect(boundsRect) && boundsRect != boundsStateFlow.value) {
-              boundsStateFlow.value = Rect(boundsRect)
-            }
+            view.getScreenRect(boundsRect)
+            if (boundsRect != boundsStateFlow.value) boundsStateFlow.value = Rect(boundsRect)
           }
 
           override fun onViewAttachedToWindow(v: View) {

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/ScreenOverlayDialogFactory.kt

@@ -5,6 +5,7 @@ import android.content.Context
 import android.graphics.drawable.ColorDrawable
 import android.util.TypedValue
 import android.view.Window
+import android.view.WindowManager.LayoutParams.FLAG_DIM_BEHIND
 import android.view.WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL
 import com.squareup.workflow1.ui.Screen
 import com.squareup.workflow1.ui.ScreenViewHolder
@@ -135,19 +136,22 @@ public open class ScreenOverlayDialogFactory<S : Screen, O : ScreenOverlay<S>>(
       // Note that we always tell Android to make the window non-modal, regardless of our own
       // notion of its modality. Even a modal dialog should only block events within
       // the appropriate bounds, but Android makes them block everywhere.
-      window.setFlags(FLAG_NOT_TOUCH_MODAL, FLAG_NOT_TOUCH_MODAL)
+      window.addFlags(FLAG_NOT_TOUCH_MODAL)
     }
   }
 }
 
 /**
- * The default implementation of [ScreenOverlayDialogFactory.buildDialogWithContent].
+ * Used by the default implementation of [ScreenOverlayDialogFactory.buildDialogWithContent]
+ * to show [contentHolder].
  *
  * - Makes the receiver [non-cancelable][Dialog.setCancelable]
  *
  * - Sets the [background][Window.setBackgroundDrawable] of the receiver's [Window] based
  *   on its theme, if any, or else `null`. (Setting the background to `null` ensures the window
  *   can go full bleed.)
+ *
+ * - Disables dimming.
  */
 @OptIn(WorkflowUiExperimentalApi::class)
 public fun Dialog.setContent(contentHolder: ScreenViewHolder<*>) {
@@ -163,7 +167,11 @@ public fun Dialog.setContent(contentHolder: ScreenViewHolder<*>) {
     if (maybeWindowColor.type in TypedValue.TYPE_FIRST_COLOR_INT..TypedValue.TYPE_LAST_COLOR_INT) {
       ColorDrawable(maybeWindowColor.data)
     } else {
-      ColorDrawable(contentHolder.view.resources.getColor(android.R.color.white))
+      // If we don't at least set it to null, the window cannot go full bleed.
+      null
     }
-  window!!.setBackgroundDrawable(background)
+  with(window!!) {
+    setBackgroundDrawable(background)
+    clearFlags(FLAG_DIM_BEHIND)
+  }
 }

workflow-ui/core-common/api/core-common.api

@@ -216,11 +216,13 @@ public final class com/squareup/workflow1/ui/container/BackStackScreenKt {
 	public static final fun toBackStackScreenOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen;
 }
 
-public final class com/squareup/workflow1/ui/container/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Screen {
-	public fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;)V
-	public synthetic fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+public final class com/squareup/workflow1/ui/container/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Screen {
+	public fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;Ljava/lang/String;)V
+	public synthetic fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun <init> (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/container/Overlay;)V
 	public final fun getBody ()Lcom/squareup/workflow1/ui/Screen;
+	public fun getCompatibilityKey ()Ljava/lang/String;
+	public final fun getName ()Ljava/lang/String;
 	public final fun getOverlays ()Ljava/util/List;
 	public final fun mapBody (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/container/BodyAndOverlaysScreen;
 	public final fun mapModals (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/container/BodyAndOverlaysScreen;

workflow-ui/core-common/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysScreen.kt

@@ -4,7 +4,6 @@ import com.squareup.workflow1.ui.Compatible
 import com.squareup.workflow1.ui.Compatible.Companion.keyFor
 import com.squareup.workflow1.ui.Screen
 import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
-import com.squareup.workflow1.ui.Wrapper
 
 /**
  * A screen that may stack a number of [Overlay]s over a body.