Skip to content

Commit d8ecc32

Browse files
rjrjrrjrjr
committed
Introduces OverlayDialogHolder.onUpdateBounds
`OverlayDialogHolder.onUpdateBounds` replaces `ScreenOverlayDialogFactory.updateBounds`. I finally tried to use `ScreenOverlayDialogFactory.updateBounds` and was reminded once again why it's bad to create interfaces that build something and then take it back later to be updated. If you want the renderings to be involved in setting the bounds policy, there was no way to smuggle that information along with the Dialog returned from `buildDialogWithContent`, so that `updateBounds` could honor it. Also, it was always pretty ugly that the bounds hook was special to the `ScreenOverlay` world. The fix is make bounds maintenance part of the job of the `OverlayDialogHolder` interface. Square reviewers will notice that we're now an even more faithful rip off of Mosaic's DialogRunner -- hi @helios175.
1 parent d19ae7a commit d8ecc32

File tree

11 files changed

+123
-100
lines changed

11 files changed

+123
-100
lines changed

‎samples/containers/android/src/main/java/com/squareup/sample/container/panel/PanelOverlayDialogFactory.kt‎

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@ import android.graphics.Rect
55
import com.squareup.sample.container.R
66
import com.squareup.workflow1.ui.Screen
77
import com.squareup.workflow1.ui.ScreenViewHolder
8+
import com.squareup.workflow1.ui.ViewEnvironment
89
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
10+
import com.squareup.workflow1.ui.container.OverlayDialogHolder
911
import com.squareup.workflow1.ui.container.ScreenOverlayDialogFactory
12+
import com.squareup.workflow1.ui.container.setBounds
1013
import com.squareup.workflow1.ui.container.setContent
14+
import com.squareup.workflow1.ui.show
1115

1216
/**
1317
* Android support for [PanelOverlay].
@@ -18,41 +22,48 @@ internal object PanelOverlayDialogFactory :
1822
type = PanelOverlay::class
1923
) {
2024
/**
21-
* Forks the default implementation to apply [R.style.PanelDialog], for
22-
* enter and exit animation.
25+
* Forks the default implementation to apply [R.style.PanelDialog] for
26+
* enter and exit animation, and to customize [bounds][OverlayDialogHolder.onUpdateBounds].
2327
*/
24-
override fun buildDialogWithContent(content: ScreenViewHolder<Screen>): Dialog {
25-
return Dialog(content.view.context, R.style.PanelDialog).also {
26-
it.setContent(content)
27-
}
28-
}
28+
override fun buildDialogWithContent(
29+
initialRendering: PanelOverlay<Screen>,
30+
initialEnvironment: ViewEnvironment,
31+
content: ScreenViewHolder<Screen>
32+
): OverlayDialogHolder<PanelOverlay<Screen>> {
33+
val dialog = Dialog(content.view.context, R.style.PanelDialog)
34+
dialog.setContent(content)
2935

30-
override fun updateBounds(
31-
dialog: Dialog,
32-
bounds: Rect
33-
) {
34-
val refinedBounds: Rect = if (!dialog.context.isTablet) {
35-
// On a phone, fill the bounds entirely.
36-
bounds
37-
} else {
38-
if (bounds.height() > bounds.width()) {
39-
val margin = bounds.height() - bounds.width()
40-
val topDelta = margin / 2
41-
val bottomDelta = margin - topDelta
42-
Rect(bounds).apply {
43-
top = bounds.top + topDelta
44-
bottom = bounds.bottom - bottomDelta
45-
}
46-
} else {
47-
val margin = bounds.width() - bounds.height()
48-
val leftDelta = margin / 2
49-
val rightDelta = margin - leftDelta
50-
Rect(bounds).apply {
51-
left = bounds.left + leftDelta
52-
right = bounds.right - rightDelta
36+
return OverlayDialogHolder(
37+
initialEnvironment = initialEnvironment,
38+
dialog = dialog,
39+
onUpdateBounds = { bounds ->
40+
val refinedBounds: Rect = if (!dialog.context.isTablet) {
41+
// On a phone, fill the bounds entirely.
42+
bounds
43+
} else {
44+
if (bounds.height() > bounds.width()) {
45+
val margin = bounds.height() - bounds.width()
46+
val topDelta = margin / 2
47+
val bottomDelta = margin - topDelta
48+
Rect(bounds).apply {
49+
top = bounds.top + topDelta
50+
bottom = bounds.bottom - bottomDelta
51+
}
52+
} else {
53+
val margin = bounds.width() - bounds.height()
54+
val leftDelta = margin / 2
55+
val rightDelta = margin - leftDelta
56+
Rect(bounds).apply {
57+
left = bounds.left + leftDelta
58+
right = bounds.right - rightDelta
59+
}
60+
}
5361
}
62+
63+
dialog.setBounds(refinedBounds)
5464
}
65+
) { overlayRendering, environment ->
66+
content.show(overlayRendering.content, environment)
5567
}
56-
super.updateBounds(dialog, refinedBounds)
5768
}
5869
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.squareup.workflow1.ui.compose
22

3-
import android.app.Dialog
43
import android.content.Context
54
import android.view.View
65
import android.view.ViewGroup
@@ -569,9 +568,6 @@ internal class ComposeViewTreeIntegrationTest {
569568
override val dialogFactory = object : ScreenOverlayDialogFactory<Screen, TestModal>(
570569
TestModal::class
571570
) {
572-
override fun buildDialogWithContent(content: ScreenViewHolder<Screen>): Dialog {
573-
return Dialog(content.view.context).apply { setContentView(content.view) }
574-
}
575571
}
576572
}
577573

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,8 @@ public class com/squareup/workflow1/ui/container/AlertOverlayDialogFactory : com
436436
}
437437

438438
public final class com/squareup/workflow1/ui/container/AndroidDialogBoundsKt {
439-
public static final fun maintainBounds (Landroid/app/Dialog;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function2;)V
440-
public static final fun maintainBounds (Landroid/app/Dialog;Lkotlinx/coroutines/flow/StateFlow;Lkotlin/jvm/functions/Function2;)V
439+
public static final fun maintainBounds (Landroid/app/Dialog;Lcom/squareup/workflow1/ui/ViewEnvironment;Lkotlin/jvm/functions/Function1;)V
440+
public static final fun maintainBounds (Landroid/app/Dialog;Lkotlinx/coroutines/flow/StateFlow;Lkotlin/jvm/functions/Function1;)V
441441
public static final fun setBounds (Landroid/app/Dialog;Landroid/graphics/Rect;)V
442442
}
443443

@@ -672,6 +672,7 @@ public abstract interface class com/squareup/workflow1/ui/container/OverlayDialo
672672
public static final field Companion Lcom/squareup/workflow1/ui/container/OverlayDialogHolder$Companion;
673673
public abstract fun getDialog ()Landroid/app/Dialog;
674674
public abstract fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
675+
public abstract fun getOnUpdateBounds ()Lkotlin/jvm/functions/Function1;
675676
public abstract fun getRunner ()Lkotlin/jvm/functions/Function2;
676677
}
677678

@@ -689,16 +690,18 @@ public final class com/squareup/workflow1/ui/container/OverlayDialogHolder$Compa
689690
}
690691

691692
public final class com/squareup/workflow1/ui/container/OverlayDialogHolderKt {
692-
public static final fun OverlayDialogHolder (Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
693+
public static final fun OverlayDialogHolder (Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
694+
public static synthetic fun OverlayDialogHolder$default (Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
693695
public static final fun canShow (Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;Lcom/squareup/workflow1/ui/container/Overlay;)Z
694696
public static final fun getShowing (Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;)Lcom/squareup/workflow1/ui/container/Overlay;
695697
public static final fun show (Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;Lcom/squareup/workflow1/ui/container/Overlay;Lcom/squareup/workflow1/ui/ViewEnvironment;)V
696698
}
697699

698700
public final class com/squareup/workflow1/ui/container/RealOverlayDialogHolder : com/squareup/workflow1/ui/container/OverlayDialogHolder {
699-
public fun <init> (Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Lkotlin/jvm/functions/Function2;)V
701+
public fun <init> (Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/app/Dialog;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function2;)V
700702
public fun getDialog ()Landroid/app/Dialog;
701703
public fun getEnvironment ()Lcom/squareup/workflow1/ui/ViewEnvironment;
704+
public fun getOnUpdateBounds ()Lkotlin/jvm/functions/Function1;
702705
public fun getRunner ()Lkotlin/jvm/functions/Function2;
703706
}
704707

@@ -707,9 +710,8 @@ public class com/squareup/workflow1/ui/container/ScreenOverlayDialogFactory : co
707710
public fun buildContent (Lcom/squareup/workflow1/ui/ScreenViewFactory;Lcom/squareup/workflow1/ui/Screen;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;)Lcom/squareup/workflow1/ui/ScreenViewHolder;
708711
public synthetic fun buildDialog (Lcom/squareup/workflow1/ui/container/Overlay;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
709712
public final fun buildDialog (Lcom/squareup/workflow1/ui/container/ScreenOverlay;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
710-
public fun buildDialogWithContent (Lcom/squareup/workflow1/ui/ScreenViewHolder;)Landroid/app/Dialog;
713+
public fun buildDialogWithContent (Lcom/squareup/workflow1/ui/container/ScreenOverlay;Lcom/squareup/workflow1/ui/ViewEnvironment;Lcom/squareup/workflow1/ui/ScreenViewHolder;)Lcom/squareup/workflow1/ui/container/OverlayDialogHolder;
711714
public fun getType ()Lkotlin/reflect/KClass;
712-
public fun updateBounds (Landroid/app/Dialog;Landroid/graphics/Rect;)V
713715
}
714716

715717
public final class com/squareup/workflow1/ui/container/ScreenOverlayDialogFactoryKt {

‎workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ internal class DialogIntegrationTest {
6666
}
6767

6868
override fun buildDialogWithContent(
69+
initialRendering: DialogRendering,
70+
initialEnvironment: ViewEnvironment,
6971
content: ScreenViewHolder<ContentRendering>
70-
) = super.buildDialogWithContent(content).also { latestDialog = it }
72+
): OverlayDialogHolder<DialogRendering> =
73+
super.buildDialogWithContent(initialRendering, initialEnvironment, content).also {
74+
latestDialog = it.dialog
75+
}
7176
}
7277
}
7378

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ public open class AlertOverlayDialogFactory : OverlayDialogFactory<AlertOverlay>
5252
alertDialog.setButton(button.toId(), " ") { _, _ -> }
5353
}
5454

55-
OverlayDialogHolder(initialEnvironment, alertDialog) { rendering, _ ->
55+
OverlayDialogHolder(
56+
initialEnvironment = initialEnvironment,
57+
dialog = alertDialog,
58+
onUpdateBounds = null
59+
) { rendering, _ ->
5660
with(alertDialog) {
5761
if (rendering.cancelable) {
5862
setOnCancelListener { rendering.onEvent(Canceled) }

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import kotlinx.coroutines.flow.onEach
1919
* [bounds] is expected to be in global display coordinates,
2020
* e.g. as returned from [View.getGlobalVisibleRect].
2121
*
22-
* @see ScreenOverlayDialogFactory.updateBounds
22+
* @see OverlayDialogHolder.onUpdateBounds
2323
*/
2424
@WorkflowUiExperimentalApi
2525
public fun Dialog.setBounds(bounds: Rect) {
@@ -37,23 +37,23 @@ public fun Dialog.setBounds(bounds: Rect) {
3737
@WorkflowUiExperimentalApi
3838
internal fun <D : Dialog> D.maintainBounds(
3939
environment: ViewEnvironment,
40-
onBoundsChange: (D, Rect) -> Unit
40+
onBoundsChange: (Rect) -> Unit
4141
) {
4242
maintainBounds(environment[OverlayArea].bounds, onBoundsChange)
4343
}
4444

4545
@WorkflowUiExperimentalApi
4646
internal fun <D : Dialog> D.maintainBounds(
4747
bounds: StateFlow<Rect>,
48-
onBoundsChange: (D, Rect) -> Unit
48+
onBoundsChange: (Rect) -> Unit
4949
) {
5050
val window = requireNotNull(window) { "Dialog must be attached to a window." }
5151
window.callback = object : Window.Callback by window.callback {
5252
var scope: CoroutineScope? = null
5353

5454
override fun onAttachedToWindow() {
5555
scope = CoroutineScope(Dispatchers.Main.immediate).also {
56-
bounds.onEach { b -> onBoundsChange(this@maintainBounds, b) }
56+
bounds.onEach { b -> onBoundsChange(b) }
5757
.launchIn(it)
5858
}
5959
}
@@ -65,5 +65,5 @@ internal fun <D : Dialog> D.maintainBounds(
6565
}
6666

6767
// If already attached, set the bounds eagerly.
68-
if (window.peekDecorView()?.isAttachedToWindow == true) onBoundsChange(this, bounds.value)
68+
if (window.peekDecorView()?.isAttachedToWindow == true) onBoundsChange(bounds.value)
6969
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal class DialogSession(
2626
index: Int,
2727
holder: OverlayDialogHolder<Overlay>
2828
) {
29-
// Note similar code in LayeredDialogs
29+
// Note similar code in LayeredDialogSessions
3030
private var allowEvents = true
3131
set(value) {
3232
val was = field

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ import kotlinx.coroutines.flow.StateFlow
2828
* layouts if [BodyAndOverlaysScreen] or the default [BodyAndOverlaysContainer] bound
2929
* to it are too restrictive.
3030
*
31-
* Provides an [allowEvents] field that reflects the presence or absence of Dialogs driven
32-
* by [ModalOverlay], and makes [OverlayArea] available in the [ViewEnvironment],
33-
* which in turn drives calls to [ScreenOverlayDialogFactory.updateBounds].
31+
* - Provides an [allowEvents] field that reflects the presence or absence of Dialogs driven
32+
* by [ModalOverlay]
3433
*
35-
* Provides a [ViewTreeLifecycleOwner] per managed Dialog, and view persistence support,
36-
* both for classic [View.onSaveInstanceState] and
37-
* Jetpack [SavedStateRegistry][androidx.savedstate.SavedStateRegistry].
34+
* - Makes [OverlayArea] available in the [ViewEnvironment],
35+
* and uses it to drive calls to [OverlayDialogHolder.onUpdateBounds].
36+
*
37+
* - Provides a [ViewTreeLifecycleOwner] per managed Dialog, and view persistence support,
38+
* both for classic [View.onSaveInstanceState] and
39+
* Jetpack [SavedStateRegistry][androidx.savedstate.SavedStateRegistry].
3840
*
3941
* ## Lifecycle of a managed [Dialog][android.app.Dialog]
4042
*
@@ -72,7 +74,7 @@ import kotlinx.coroutines.flow.StateFlow
7274
*
7375
* @param bounds made available to managed dialogs via the [OverlayArea]
7476
* [ViewEnvironmentKey][com.squareup.workflow1.ui.ViewEnvironmentKey],
75-
* which drives [ScreenOverlayDialogFactory.updateBounds].
77+
* which drives [OverlayDialogHolder.onUpdateBounds].
7678
*
7779
* @param cancelEvents function to be called when a modal session starts -- that is,
7880
* when [update] is first called with a [ModalOverlay] member, or called again with
@@ -163,6 +165,9 @@ public class LayeredDialogSessions private constructor(
163165
overlay.toDialogFactory(dialogEnv)
164166
.buildDialog(overlay, dialogEnv, context)
165167
.let { holder ->
168+
holder.onUpdateBounds?.let { updateBounds ->
169+
holder.dialog.maintainBounds(holder.environment) { b -> updateBounds(b) }
170+
}
166171
DialogSession(i, holder).also { newSession ->
167172
// Prime the pump, make the first call to OverlayDialog.show to update
168173
// the new dialog to reflect the first rendering.
@@ -286,7 +291,7 @@ public class LayeredDialogSessions private constructor(
286291
context = view.context,
287292
bounds = bounds,
288293
cancelEvents = {
289-
// Note similar code in DialogHolder.
294+
// Note similar code in DialogSession.
290295

291296
// https://stackoverflow.com/questions/2886407/dealing-with-rapid-tapping-on-buttons
292297
// If any motion events were enqueued on the main thread, cancel them.

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.squareup.workflow1.ui.container
22

33
import android.app.Dialog
4+
import android.graphics.Rect
45
import com.squareup.workflow1.ui.Screen
56
import com.squareup.workflow1.ui.ViewEnvironment
67
import com.squareup.workflow1.ui.ViewEnvironmentKey
@@ -29,6 +30,20 @@ public interface OverlayDialogHolder<in OverlayT : Overlay> {
2930
*/
3031
public val runner: (rendering: OverlayT, environment: ViewEnvironment) -> Unit
3132

33+
/**
34+
* Optional function called to report the bounds of the managing container view,
35+
* as reported by [OverlayArea]. Well behaved [Overlay] dialogs are expected to
36+
* be restricted to those bounds, to the extent practical -- you probably want to ignore
37+
* this for AlertDialog, e.g.
38+
*
39+
* Honoring this contract makes it easy to define areas of the display
40+
* that are outside of the "shadow" of a modal dialog. Imagine an app
41+
* with a status bar that should not be covered by modals.
42+
*
43+
* Default implementation provided by the factory function below calls [Dialog.setBounds].
44+
*/
45+
public val onUpdateBounds: ((Rect) -> Unit)?
46+
3247
public companion object {
3348
/**
3449
* Default value returned for the [InOverlay] [ViewEnvironmentKey], and therefore the
@@ -87,7 +102,8 @@ public val OverlayDialogHolder<*>.showing: Overlay
87102
public fun <OverlayT : Overlay> OverlayDialogHolder(
88103
initialEnvironment: ViewEnvironment,
89104
dialog: Dialog,
105+
onUpdateBounds: ((Rect) -> Unit)? = { dialog.setBounds(it) },
90106
runner: (rendering: OverlayT, environment: ViewEnvironment) -> Unit
91107
): OverlayDialogHolder<OverlayT> {
92-
return RealOverlayDialogHolder(initialEnvironment, dialog, runner)
108+
return RealOverlayDialogHolder(initialEnvironment, dialog, onUpdateBounds, runner)
93109
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package com.squareup.workflow1.ui.container
22

33
import android.app.Dialog
4+
import android.graphics.Rect
45
import com.squareup.workflow1.ui.ViewEnvironment
56
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
67

78
@WorkflowUiExperimentalApi
89
internal class RealOverlayDialogHolder<OverlayT : Overlay>(
910
initialEnvironment: ViewEnvironment,
1011
override val dialog: Dialog,
12+
override val onUpdateBounds: ((Rect) -> Unit)?,
1113
runnerFunction: (rendering: OverlayT, environment: ViewEnvironment) -> Unit
1214
) : OverlayDialogHolder<OverlayT> {
1315

0 commit comments

Comments
 (0)