Skip to content

Commit 0639dd8

Browse files
rjrjrrjrjr
committed
Makes OverlayDialogFactory more consistent with ScreenViewFactory.
`OverlayDialogFactory` now has one method that produces `OverlayDialogHolder` wrappers, just like `ScreenViewFactory` and `ScreenViewHolder`. I'm sad about all the copy/paste API here, but every time I try to consolidate things into a single set of `UiFactory` / `UiHolder` / `UiRunner` interfaces, I wind up with an opaque mess of parameter types that usually breaks things. A `Dialog` is not a `View`, both classes are dying anyway one hopes, and my attempts to factor out a pretty shared wrapper API for them has been a really bad use of time. Also note: - Implementors of `ScreenOverlayDialogFactory` now have nearly complete control over the `ScreenViewFactory` that builds the content view. - `ModalScreenOverlayOnBackPressed` is now `ModalScreenOverlayBackButtonHelper`, and eliminates all direct dependencies on Jetpack's `OnBackPressedDispatcherOwner` -- necessary for we poor souls who remain incompatible with that. - We now take care only to apply the back button work for modals. We sure don't want toasts intercepting the back button, or anything else. This commit avoids renaming anything interesting, for better diffs and ease of review. Closes #713.
1 parent 73edd7e commit 0639dd8

File tree

13 files changed

+478
-288
lines changed

13 files changed

+478
-288
lines changed

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,28 @@ package com.squareup.sample.container.panel
22

33
import android.app.Dialog
44
import android.graphics.Rect
5-
import android.view.View
65
import com.squareup.sample.container.R
6+
import com.squareup.workflow1.ui.Screen
7+
import com.squareup.workflow1.ui.ScreenViewHolder
78
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
89
import com.squareup.workflow1.ui.container.ScreenOverlayDialogFactory
9-
import com.squareup.workflow1.ui.container.setModalContent
10+
import com.squareup.workflow1.ui.container.setContent
1011

1112
/**
1213
* Android support for [PanelOverlay].
1314
*/
1415
@OptIn(WorkflowUiExperimentalApi::class)
15-
internal object PanelOverlayDialogFactory : ScreenOverlayDialogFactory<PanelOverlay<*>>(
16-
type = PanelOverlay::class
17-
) {
16+
internal object PanelOverlayDialogFactory :
17+
ScreenOverlayDialogFactory<Screen, PanelOverlay<Screen>>(
18+
type = PanelOverlay::class
19+
) {
1820
/**
1921
* Forks the default implementation to apply [R.style.PanelDialog], for
2022
* enter and exit animation.
2123
*/
22-
override fun buildDialogWithContentView(contentView: View): Dialog {
23-
return Dialog(contentView.context, R.style.PanelDialog).also {
24-
it.setModalContent(contentView)
24+
override fun buildDialogWithContent(content: ScreenViewHolder<Screen>): Dialog {
25+
return Dialog(content.view.context, R.style.PanelDialog).also {
26+
it.setContent(content)
2527
}
2628
}
2729

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,11 +566,11 @@ internal class ComposeViewTreeIntegrationTest {
566566
data class TestModal(
567567
override val content: Screen
568568
) : ScreenOverlay<Screen>, AndroidOverlay<TestModal> {
569-
override val dialogFactory = object : ScreenOverlayDialogFactory<TestModal>(
569+
override val dialogFactory = object : ScreenOverlayDialogFactory<Screen, TestModal>(
570570
TestModal::class
571571
) {
572-
override fun buildDialogWithContentView(contentView: View): Dialog {
573-
return Dialog(contentView.context).apply { setContentView(contentView) }
572+
override fun buildDialogWithContent(content: ScreenViewHolder<Screen>): Dialog {
573+
return Dialog(content.view.context).apply { setContentView(content.view) }
574574
}
575575
}
576576
}

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

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

33
import android.app.AlertDialog
4-
import android.app.Dialog
54
import android.content.Context
65
import android.content.DialogInterface
76
import android.view.View.GONE
@@ -30,7 +29,7 @@ public open class AlertOverlayDialogFactory : OverlayDialogFactory<AlertOverlay>
3029
initialRendering: AlertOverlay,
3130
initialEnvironment: ViewEnvironment,
3231
context: Context
33-
): AlertDialog {
32+
): OverlayDialogHolder<AlertOverlay> {
3433
return AlertDialog.Builder(context, initialEnvironment[AlertDialogThemeResId])
3534
.create().apply {
3635
for (button in Button.values()) {
@@ -53,29 +52,26 @@ public open class AlertOverlayDialogFactory : OverlayDialogFactory<AlertOverlay>
5352
setButton(button.toId(), " ") { _, _ -> }
5453
}
5554
}
56-
}
57-
58-
open override fun updateDialog(
59-
dialog: Dialog,
60-
rendering: AlertOverlay,
61-
environment: ViewEnvironment
62-
) {
63-
(dialog as AlertDialog).apply {
64-
if (rendering.cancelable) {
65-
setOnCancelListener { rendering.onEvent(Canceled) }
66-
setCancelable(true)
67-
} else {
68-
setCancelable(false)
69-
}
55+
.let { alertDialog ->
56+
OverlayDialogHolder(initialEnvironment, alertDialog) { rendering, _ ->
57+
with(alertDialog) {
58+
if (rendering.cancelable) {
59+
setOnCancelListener { rendering.onEvent(Canceled) }
60+
setCancelable(true)
61+
} else {
62+
setCancelable(false)
63+
}
7064

71-
setMessage(rendering.message)
72-
setTitle(rendering.title)
65+
setMessage(rendering.message)
66+
setTitle(rendering.title)
7367

74-
// The buttons won't actually exist until the dialog is showing.
75-
if (isShowing) updateButtonsOnShow(rendering) else setOnShowListener {
76-
updateButtonsOnShow(rendering)
68+
// The buttons won't actually exist until the dialog is showing.
69+
if (isShowing) updateButtonsOnShow(rendering) else setOnShowListener {
70+
updateButtonsOnShow(rendering)
71+
}
72+
}
73+
}
7774
}
78-
}
7975
}
8076

8177
protected fun Button.toId(): Int = when (this) {

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

Lines changed: 71 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.squareup.workflow1.ui.container
22

3-
import android.app.Dialog
4-
import android.content.Context
53
import android.os.Bundle
64
import android.os.Parcel
75
import android.os.Parcelable
@@ -14,41 +12,26 @@ import androidx.core.view.doOnDetach
1412
import androidx.lifecycle.DefaultLifecycleObserver
1513
import androidx.lifecycle.LifecycleOwner
1614
import com.squareup.workflow1.ui.Compatible
17-
import com.squareup.workflow1.ui.ViewEnvironment
1815
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
1916
import com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner
2017
import com.squareup.workflow1.ui.androidx.WorkflowSavedStateRegistryAggregator
21-
import com.squareup.workflow1.ui.compatible
2218

2319
/**
24-
* Used by [LayeredDialogs] to keep a [Dialog] tied to its [rendering] and [environment].
20+
* Used by [LayeredDialogs] to manage lifecycle and view persistence concerns for an
21+
* [OverlayDialogHolder], as well as enforcing modal behavior.
2522
*/
2623
@WorkflowUiExperimentalApi
27-
internal class DialogHolder<T : Overlay>(
28-
initialRendering: T,
29-
initialViewEnvironment: ViewEnvironment,
24+
// TODO Rename DialogSession
25+
internal class DialogHolder(
3026
index: Int,
31-
private val context: Context,
32-
private val factory: OverlayDialogFactory<T>
27+
holder: OverlayDialogHolder<Overlay>
3328
) {
34-
val savedStateRegistryKey = Compatible.keyFor(initialRendering, index.toString())
35-
36-
var rendering: T = initialRendering
37-
private set
38-
39-
var environment: ViewEnvironment = initialViewEnvironment
40-
private set
41-
42-
private val modal = initialRendering is ModalOverlay
43-
44-
private var dialogOrNull: Dialog? = null
45-
4629
// Note similar code in LayeredDialogs
4730
private var allowEvents = true
4831
set(value) {
4932
val was = field
5033
field = value
51-
dialogOrNull?.window?.takeIf { value != was }?.let { window ->
34+
holder.dialog.window?.takeIf { value != was }?.let { window ->
5235
// https://stackoverflow.com/questions/2886407/dealing-with-rapid-tapping-on-buttons
5336
// If any motion events were enqueued on the main thread, cancel them.
5437
dispatchCancelEvent { window.superDispatchTouchEvent(it) }
@@ -59,61 +42,73 @@ internal class DialogHolder<T : Overlay>(
5942
}
6043
}
6144

62-
fun show(
45+
/**
46+
* Wrap the given dialog holder to maintain [allowEvents] on each update.
47+
*/
48+
val holder: OverlayDialogHolder<Overlay> = OverlayDialogHolder(
49+
holder.environment, holder.dialog
50+
) { overlay, environment ->
51+
allowEvents = !environment[CoveredByModal]
52+
holder.show(overlay, environment)
53+
}
54+
55+
val savedStateRegistryKey = Compatible.keyFor(holder.showing, index.toString())
56+
57+
fun showDialog(
6358
parentLifecycleOwner: LifecycleOwner,
6459
stateRegistryAggregator: WorkflowSavedStateRegistryAggregator
6560
) {
66-
requireDialog().let { dialog ->
67-
dialog.window?.let { window ->
68-
val realWindowCallback = window.callback
69-
window.callback = object : Window.Callback by realWindowCallback {
70-
override fun dispatchTouchEvent(event: MotionEvent): Boolean {
71-
return !allowEvents || realWindowCallback.dispatchTouchEvent(event)
72-
}
61+
val dialog = holder.dialog
7362

74-
override fun dispatchKeyEvent(event: KeyEvent): Boolean {
75-
return !allowEvents || realWindowCallback.dispatchKeyEvent(event)
76-
}
63+
dialog.window?.let { window ->
64+
val realWindowCallback = window.callback
65+
window.callback = object : Window.Callback by realWindowCallback {
66+
override fun dispatchTouchEvent(event: MotionEvent): Boolean {
67+
return !allowEvents || realWindowCallback.dispatchTouchEvent(event)
68+
}
69+
70+
override fun dispatchKeyEvent(event: KeyEvent): Boolean {
71+
return !allowEvents || realWindowCallback.dispatchKeyEvent(event)
7772
}
7873
}
74+
}
7975

80-
dialog.show()
81-
dialog.window?.decorView?.also { decorView ->
82-
// Implementations of buildDialog may set their own WorkflowLifecycleOwner on the
83-
// content view, so to avoid interfering with them we also set it here. When the views
84-
// are attached, this will become the parent lifecycle of the one from buildDialog if
85-
// any, and so we can use our lifecycle to destroy-on-detach the dialog hierarchy.
86-
WorkflowLifecycleOwner.installOn(
87-
decorView,
88-
findParentLifecycle = { parentLifecycleOwner.lifecycle }
89-
)
90-
// Ensure that each dialog has its own ViewTreeSavedStateRegistryOwner,
91-
// so views in each dialog layer don't clash with other layers.
92-
stateRegistryAggregator.installChildRegistryOwnerOn(
93-
view = decorView,
94-
key = savedStateRegistryKey
95-
)
96-
97-
decorView.doOnAttach {
98-
val lifecycle = parentLifecycleOwner.lifecycle
99-
val onDestroy = object : DefaultLifecycleObserver {
100-
override fun onDestroy(owner: LifecycleOwner) {
101-
dismiss()
102-
}
76+
dialog.show()
77+
dialog.window?.decorView?.also { decorView ->
78+
// Implementations of buildDialog may set their own WorkflowLifecycleOwner on the
79+
// content view, so to avoid interfering with them we also set it here. When the views
80+
// are attached, this will become the parent lifecycle of the one from buildDialog if
81+
// any, and so we can use our lifecycle to destroy-on-detach the dialog hierarchy.
82+
WorkflowLifecycleOwner.installOn(
83+
decorView,
84+
findParentLifecycle = { parentLifecycleOwner.lifecycle }
85+
)
86+
// Ensure that each dialog has its own ViewTreeSavedStateRegistryOwner,
87+
// so views in each dialog layer don't clash with other layers.
88+
stateRegistryAggregator.installChildRegistryOwnerOn(
89+
view = decorView,
90+
key = savedStateRegistryKey
91+
)
92+
93+
decorView.doOnAttach {
94+
val lifecycle = parentLifecycleOwner.lifecycle
95+
val onDestroy = object : DefaultLifecycleObserver {
96+
override fun onDestroy(owner: LifecycleOwner) {
97+
dismiss()
10398
}
104-
105-
// Android makes a lot of logcat noise if it has to close the window for us. :/
106-
// And no, we can't call ref.dismiss() directly from the doOnDetach lambda,
107-
// that's too late.
108-
// https://github.com/square/workflow/issues/51
109-
lifecycle.addObserver(onDestroy)
110-
111-
// Note that we are careful not to make the doOnDetach call unless
112-
// we actually get attached. It is common for the dialog to be dismissed
113-
// before it is ever shown, so doOnDetach would never fire and we'd leak the
114-
// onDestroy lambda.
115-
decorView.doOnDetach { lifecycle.removeObserver(onDestroy) }
11699
}
100+
101+
// Android makes a lot of logcat noise if it has to close the window for us. :/
102+
// And no, we can't call ref.dismiss() directly from the doOnDetach lambda,
103+
// that's too late.
104+
// https://github.com/square/workflow/issues/51
105+
lifecycle.addObserver(onDestroy)
106+
107+
// Note that we are careful not to make the doOnDetach call unless
108+
// we actually get attached. It is common for the dialog to be dismissed
109+
// before it is ever shown, so doOnDetach would never fire and we'd leak the
110+
// onDestroy lambda.
111+
decorView.doOnDetach { lifecycle.removeObserver(onDestroy) }
117112
}
118113
}
119114
}
@@ -122,54 +117,23 @@ internal class DialogHolder<T : Overlay>(
122117
// The dialog's views are about to be detached, and when that happens we want to transition
123118
// the dialog view's lifecycle to a terminal state even though the parent is probably still
124119
// alive.
125-
dialogOrNull?.let { dialog ->
126-
dialog.window?.decorView?.let(WorkflowLifecycleOwner::get)?.destroyOnDetach()
127-
dialog.dismiss()
128-
}
129-
}
130-
131-
fun canTakeRendering(rendering: Overlay): Boolean {
132-
return compatible(this.rendering, rendering)
133-
}
134-
135-
fun takeRendering(
136-
rendering: Overlay,
137-
environment: ViewEnvironment
138-
) {
139-
check(canTakeRendering(rendering)) {
140-
"Expected $this to be able to show rendering $rendering, but that did not match " +
141-
"previous rendering ${this.rendering}."
142-
}
143-
144-
@Suppress("UNCHECKED_CAST")
145-
this.rendering = rendering as T
146-
this.environment = environment
147-
allowEvents = !modal || !environment[CoveredByModal]
148-
149-
dialogOrNull?.let { dialog ->
150-
factory.updateDialog(dialog, this.rendering, this.environment)
120+
with(holder.dialog) {
121+
window?.decorView?.let(WorkflowLifecycleOwner::get)?.destroyOnDetach()
122+
dismiss()
151123
}
152124
}
153125

154126
internal fun save(): KeyAndBundle? {
155-
val saved = dialogOrNull?.window?.saveHierarchyState() ?: return null
156-
return KeyAndBundle(Compatible.keyFor(rendering), saved)
127+
val saved = holder.dialog.window?.saveHierarchyState() ?: return null
128+
return KeyAndBundle(Compatible.keyFor(holder.showing), saved)
157129
}
158130

159131
internal fun restore(keyAndBundle: KeyAndBundle) {
160-
if (Compatible.keyFor(rendering) == keyAndBundle.compatibilityKey) {
161-
requireDialog().window?.restoreHierarchyState(keyAndBundle.bundle)
132+
if (Compatible.keyFor(holder.showing) == keyAndBundle.compatibilityKey) {
133+
holder.dialog.window?.restoreHierarchyState(keyAndBundle.bundle)
162134
}
163135
}
164136

165-
private fun requireDialog(): Dialog {
166-
return dialogOrNull ?: factory.buildDialog(rendering, environment, context)
167-
.also {
168-
dialogOrNull = it
169-
takeRendering(rendering, environment)
170-
}
171-
}
172-
173137
internal data class KeyAndBundle(
174138
internal val compatibilityKey: String,
175139
internal val bundle: Bundle

0 commit comments

Comments
 (0)