Skip to content

Commit a8d9a86

Browse files
committed
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.
1 parent fbfda50 commit a8d9a86

File tree

7 files changed

+49
-16
lines changed

7 files changed

+49
-16
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ class ButtonBar(
2222
vararg buttons: Button?,
2323
@ColorRes val color: Int = -1,
2424
) : AndroidScreen<ButtonBar> {
25-
val buttons: List<Button> = buttons.filterNotNull().toList()
25+
private val buttons: List<Button> = buttons.filterNotNull().toList()
2626

2727
override val viewFactory =
2828
ScreenViewFactory.fromCode<ButtonBar> { _, initialEnvironment, context, _ ->
2929
LinearLayout(context).let { view ->
30+
@Suppress("DEPRECATION")
3031
if (color > -1) view.background = ColorDrawable(view.resources.getColor(color))
3132

3233
view.gravity = Gravity.CENTER

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ public final class com/squareup/workflow1/ui/container/AlertOverlayDialogFactory
306306
}
307307

308308
public final class com/squareup/workflow1/ui/container/AndroidDialogBoundsKt {
309+
public static final fun getScreenRect (Landroid/view/View;Landroid/graphics/Rect;)V
309310
public static final fun setBounds (Landroid/app/Dialog;Landroid/graphics/Rect;)V
310311
}
311312

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package com.squareup.workflow1.ui.container
22

3+
import android.annotation.SuppressLint
34
import android.app.Dialog
45
import android.graphics.Rect
56
import android.view.Gravity
67
import android.view.View
78
import android.view.Window
9+
import android.view.WindowManager.LayoutParams.FLAG_LAYOUT_IN_SCREEN
810
import com.squareup.workflow1.ui.ViewEnvironment
911
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
1012
import kotlinx.coroutines.CoroutineScope
@@ -16,16 +18,19 @@ import kotlinx.coroutines.flow.onEach
1618

1719
/**
1820
* Updates the size of the [Window] of the receiving [Dialog].
19-
* [bounds] is expected to be in global display coordinates,
20-
* e.g. as returned from [View.getGlobalVisibleRect].
21+
* [bounds] is expected to be in global screen coordinates,
22+
* as returned from [View.getScreenRect].
2123
*
2224
* @see OverlayDialogHolder.onUpdateBounds
2325
*/
2426
@WorkflowUiExperimentalApi
2527
public fun Dialog.setBounds(bounds: Rect) {
2628
window?.let {
29+
it.addFlags(FLAG_LAYOUT_IN_SCREEN)
2730
it.attributes = it.attributes.apply {
28-
gravity = Gravity.TOP or Gravity.START
31+
// Our absolute coordinates are independent from Rtl.
32+
@SuppressLint("RtlHardcoded")
33+
gravity = Gravity.LEFT or Gravity.TOP // we use absolute coordinates.
2934
width = bounds.width()
3035
height = bounds.height()
3136
x = bounds.left
@@ -34,6 +39,23 @@ public fun Dialog.setBounds(bounds: Rect) {
3439
}
3540
}
3641

42+
/**
43+
* Returns the bounds of this [View] in the coordinate space of the device
44+
* screen, based on [View.getLocationOnScreen] and its reported [width][View.getWidth]
45+
* and [height][View.getHeight].
46+
*/
47+
@WorkflowUiExperimentalApi
48+
public fun View.getScreenRect(rect: Rect) {
49+
val coordinates = IntArray(2)
50+
getLocationOnScreen(coordinates)
51+
rect.apply {
52+
left = coordinates[0]
53+
top = coordinates[1]
54+
right = left + width
55+
bottom = top + height
56+
}
57+
}
58+
3759
@WorkflowUiExperimentalApi
3860
internal fun <D : Dialog> D.maintainBounds(
3961
environment: ViewEnvironment,

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ public class LayeredDialogSessions private constructor(
279279
superDispatchTouchEvent: (MotionEvent) -> Unit
280280
): LayeredDialogSessions {
281281
val boundsRect = Rect()
282-
if (view.isAttachedToWindow) view.getGlobalVisibleRect(boundsRect)
282+
if (view.isAttachedToWindow) view.getScreenRect(boundsRect)
283283
val boundsStateFlow = MutableStateFlow(Rect(boundsRect))
284284

285285
return LayeredDialogSessions(
@@ -326,9 +326,8 @@ public class LayeredDialogSessions private constructor(
326326
// accordingly.
327327
val attachStateChangeListener = object : OnAttachStateChangeListener {
328328
val boundsListener = OnGlobalLayoutListener {
329-
if (view.getGlobalVisibleRect(boundsRect) && boundsRect != boundsStateFlow.value) {
330-
boundsStateFlow.value = Rect(boundsRect)
331-
}
329+
view.getScreenRect(boundsRect)
330+
if (boundsRect != boundsStateFlow.value) boundsStateFlow.value = Rect(boundsRect)
332331
}
333332

334333
override fun onViewAttachedToWindow(v: View) {

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import android.content.Context
55
import android.graphics.drawable.ColorDrawable
66
import android.util.TypedValue
77
import android.view.Window
8+
import android.view.WindowManager.LayoutParams.FLAG_BLUR_BEHIND
9+
import android.view.WindowManager.LayoutParams.FLAG_DIM_BEHIND
810
import android.view.WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL
911
import com.squareup.workflow1.ui.Screen
1012
import com.squareup.workflow1.ui.ScreenViewHolder
@@ -135,19 +137,22 @@ public open class ScreenOverlayDialogFactory<S : Screen, O : ScreenOverlay<S>>(
135137
// Note that we always tell Android to make the window non-modal, regardless of our own
136138
// notion of its modality. Even a modal dialog should only block events within
137139
// the appropriate bounds, but Android makes them block everywhere.
138-
window.setFlags(FLAG_NOT_TOUCH_MODAL, FLAG_NOT_TOUCH_MODAL)
140+
window.addFlags(FLAG_NOT_TOUCH_MODAL)
139141
}
140142
}
141143
}
142144

143145
/**
144-
* The default implementation of [ScreenOverlayDialogFactory.buildDialogWithContent].
146+
* Used by the default implementation of [ScreenOverlayDialogFactory.buildDialogWithContent]
147+
* to show [contentHolder].
145148
*
146149
* - Makes the receiver [non-cancelable][Dialog.setCancelable]
147150
*
148151
* - Sets the [background][Window.setBackgroundDrawable] of the receiver's [Window] based
149152
* on its theme, if any, or else `null`. (Setting the background to `null` ensures the window
150153
* can go full bleed.)
154+
*
155+
* - Disables dimming.
151156
*/
152157
@OptIn(WorkflowUiExperimentalApi::class)
153158
public fun Dialog.setContent(contentHolder: ScreenViewHolder<*>) {
@@ -163,7 +168,11 @@ public fun Dialog.setContent(contentHolder: ScreenViewHolder<*>) {
163168
if (maybeWindowColor.type in TypedValue.TYPE_FIRST_COLOR_INT..TypedValue.TYPE_LAST_COLOR_INT) {
164169
ColorDrawable(maybeWindowColor.data)
165170
} else {
166-
ColorDrawable(contentHolder.view.resources.getColor(android.R.color.white))
171+
// If we don't at least set it to null, the window cannot go full bleed.
172+
null
167173
}
168-
window!!.setBackgroundDrawable(background)
174+
with(window!!) {
175+
setBackgroundDrawable(background)
176+
clearFlags(FLAG_DIM_BEHIND)
177+
}
169178
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,13 @@ public final class com/squareup/workflow1/ui/container/BackStackScreenKt {
216216
public static final fun toBackStackScreenOrNull (Ljava/util/List;)Lcom/squareup/workflow1/ui/container/BackStackScreen;
217217
}
218218

219-
public final class com/squareup/workflow1/ui/container/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Screen {
220-
public fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;)V
221-
public synthetic fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
219+
public final class com/squareup/workflow1/ui/container/BodyAndOverlaysScreen : com/squareup/workflow1/ui/Compatible, com/squareup/workflow1/ui/Screen {
220+
public fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;Ljava/lang/String;)V
221+
public synthetic fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
222222
public fun <init> (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/container/Overlay;)V
223223
public final fun getBody ()Lcom/squareup/workflow1/ui/Screen;
224+
public fun getCompatibilityKey ()Ljava/lang/String;
225+
public final fun getName ()Ljava/lang/String;
224226
public final fun getOverlays ()Ljava/util/List;
225227
public final fun mapBody (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/container/BodyAndOverlaysScreen;
226228
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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import com.squareup.workflow1.ui.Compatible
44
import com.squareup.workflow1.ui.Compatible.Companion.keyFor
55
import com.squareup.workflow1.ui.Screen
66
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
7-
import com.squareup.workflow1.ui.Wrapper
87

98
/**
109
* A screen that may stack a number of [Overlay]s over a body.

0 commit comments

Comments
 (0)