From e5c7ff9d524ac28901ce3894f00dc44524458e9f Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Thu, 1 Jun 2023 10:30:06 -0700 Subject: [PATCH 1/2] More Dialog test coverage In anticipation of fix for #1001 --- .../nestedoverlays/NestedOverlaysAppTest.kt | 14 ++++++++ .../ui/container/DialogIntegrationTest.kt | 33 +++++++++++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt b/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt index 2dabf7bd89..83d760fded 100644 --- a/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt +++ b/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt @@ -107,6 +107,20 @@ class NestedOverlaysAppTest { .check(matches(withText("banana"))) } + @Test fun orderPreservedOnConfigChange() { + // Show the outer dialog + onTopCoverEverything().perform(click()) + + // Click the outer dialog's button to show the inner dialog. + onView(withText("Cover Body")).inRoot(isDialog()).perform(click()) + + // "Config change" + scenarioRule.scenario.recreate() + + // The green "Cover Everything" dialog is on top. + onView(withText("Reveal Body")).inRoot(isDialog()).check(matches(isDisplayed())) + } + // https://github.com/square/workflow-kotlin/issues/314 @Test fun whenBodyAndOverlaysStopsBeingRenderedDialogsAreDismissed() { onBottomCoverBody().perform(click()) diff --git a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt index b256af0711..192c0f4480 100644 --- a/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt +++ b/workflow-ui/core-android/src/androidTest/java/com/squareup/workflow1/ui/container/DialogIntegrationTest.kt @@ -1,10 +1,18 @@ package com.squareup.workflow1.ui.container import android.app.Dialog +import android.text.SpannableStringBuilder import android.view.View +import android.view.ViewGroup.LayoutParams +import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.widget.EditText import androidx.activity.ComponentActivity import androidx.lifecycle.Lifecycle.State.DESTROYED +import androidx.test.espresso.Espresso.onView +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.withText import androidx.test.ext.junit.rules.ActivityScenarioRule import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat @@ -34,8 +42,12 @@ internal class DialogIntegrationTest { ScreenViewHolder( initialRendering, EditText(context).apply { + layoutParams = LayoutParams(MATCH_PARENT, MATCH_PARENT) // Must have an id to participate in view persistence. id = 65 + // Give us something to search for so that we can be sure + // views actually get displayed + text = SpannableStringBuilder(name) } ) { _, _ -> /* Noop */ } } @@ -47,12 +59,12 @@ internal class DialogIntegrationTest { private inner class DialogRendering( name: String, override val content: ContentRendering - ) : - AndroidOverlay, ScreenOverlay { + ) : AndroidOverlay, ScreenOverlay { override fun map(transform: (ContentRendering) -> ContentU) = error("Not implemented") override val compatibilityKey = name + override val dialogFactory = object : ScreenOverlayDialogFactory( type = DialogRendering::class @@ -79,12 +91,14 @@ internal class DialogIntegrationTest { scenario.onActivity { activity -> val root = WorkflowLayout(activity) + activity.setContentView(root) root.show(screen) - - assertThat(latestContentView).isNotNull() - assertThat(latestDialog).isNotNull() - assertThat(latestDialog!!.isShowing).isTrue() } + onView(withText("content")).inRoot(isDialog()).check(matches(isDisplayed())) + + assertThat(latestContentView).isNotNull() + assertThat(latestDialog).isNotNull() + assertThat(latestDialog!!.isShowing).isTrue() } /** https://github.com/square/workflow-kotlin/issues/825 */ @@ -97,6 +111,7 @@ internal class DialogIntegrationTest { scenario.onActivity { activity -> root = WorkflowLayout(activity) + activity.setContentView(root) root.show(oneDialog) } @@ -112,6 +127,7 @@ internal class DialogIntegrationTest { val lastOverlay = latestDialog?.overlay assertThat(lastOverlay).isEqualTo(dialog2) } + onView(withText("content2")).inRoot(isDialog()).check(matches(isDisplayed())) } // Some of us are stuck with integration setups that cache @@ -129,6 +145,7 @@ internal class DialogIntegrationTest { scenario.onActivity { activity -> root = WorkflowLayout(activity) + activity.setContentView(root) root.show(oneDialog) } @@ -144,6 +161,7 @@ internal class DialogIntegrationTest { val lastOverlay = latestDialog?.overlay assertThat(lastOverlay).isEqualTo(dialog2) } + onView(withText("content2")).inRoot(isDialog()).check(matches(isDisplayed())) } @Test fun closingAnUpstreamDialogPreservesDownstream() { @@ -155,6 +173,7 @@ internal class DialogIntegrationTest { lateinit var originalDialogOne: Dialog scenario.onActivity { activity -> root = WorkflowLayout(activity) + activity.setContentView(root) root.show(showingBoth) originalDialogOne = latestDialog!! assertThat(originalDialogOne.overlayOrNull).isSameInstanceAs(overlayOne) @@ -175,8 +194,10 @@ internal class DialogIntegrationTest { scenario.onActivity { activity -> val root = WorkflowLayout(activity) + activity.setContentView(root) root.show(screen) } + onView(withText("content")).inRoot(isDialog()).check(matches(isDisplayed())) scenario.moveToState(DESTROYED) assertThat(latestDialog?.isShowing).isFalse() From bf2b9cfa6f194bf985acfca75bbc1570593bf7a5 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Wed, 31 May 2023 15:14:24 -0700 Subject: [PATCH 2/2] Do not show `Dialog`s before `Activity` content view is attached. If the very first rendering would instantiate `Dialog` windows, their content views were getting attached before the `Activity`'s content view. This would cause us to restore saved view state in the wrong order. If a `ComposeView` is in a `Dialog` in this situation, we would crash with an `IllegalStateException` from `SavedStateRegistry.consumeRestoredStateForKey`. The fix is for `LayeredDialogSessions` and `DialogCollator` to conspire to decouple created `Dialog` instances from showing them the first time. Normally we go ahead and do both at once. But if `LayeredDialogSessions.update` is called before the corresponding `BodyAndOverlaysContainer` view is attached to the `Activity` window, we defer showing until it is. Fixes #1001 --- .../nestedoverlays/NestedOverlaysAppTest.kt | 23 ++++++ .../nestedoverlays/NestedOverlaysWorkflow.kt | 48 ++++++----- workflow-ui/core-android/api/core-android.api | 2 +- .../ui/container/BodyAndOverlaysContainer.kt | 4 +- .../workflow1/ui/container/DialogCollator.kt | 21 ++++- .../workflow1/ui/container/DialogSession.kt | 39 ++++++--- .../ui/container/LayeredDialogSessions.kt | 81 +++++++++++++++---- 7 files changed, 164 insertions(+), 54 deletions(-) diff --git a/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt b/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt index 83d760fded..97f046ce27 100644 --- a/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt +++ b/samples/nested-overlays/src/androidTest/java/com/squareup/sample/nestedoverlays/NestedOverlaysAppTest.kt @@ -3,6 +3,7 @@ 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.pressBack import androidx.test.espresso.action.ViewActions.typeText import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches @@ -56,6 +57,28 @@ class NestedOverlaysAppTest { onBottomCoverEverything().assertNotDisplayed() } + @Test fun backButtonWorks() { + onTopCoverEverything().perform(click()) + onView(withText("Cover Body")).inRoot(isDialog()).perform(click()) + + onView(withText("Reveal Body")).inRoot(isDialog()).perform(pressBack()) + onView(withId(R.id.button_bar_text)).inRoot(isDialog()).perform(pressBack()) + + onTopCoverBody().assertDisplayed() + } + + @Test fun backButtonWorksAfterConfigChange() { + onTopCoverEverything().perform(click()) + onView(withText("Cover Body")).inRoot(isDialog()).perform(click()) + + scenarioRule.scenario.recreate() + + onView(withText("Reveal Body")).inRoot(isDialog()).perform(pressBack()) + onView(withId(R.id.button_bar_text)).inRoot(isDialog()).perform(pressBack()) + + onTopCoverBody().assertDisplayed() + } + // https://github.com/square/workflow-kotlin/issues/966 @Test fun canInsertDialog() { onTopCoverEverything().perform(click()) diff --git a/samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/NestedOverlaysWorkflow.kt b/samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/NestedOverlaysWorkflow.kt index 54f8d879a4..b694ed9f66 100644 --- a/samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/NestedOverlaysWorkflow.kt +++ b/samples/nested-overlays/src/main/java/com/squareup/sample/nestedoverlays/NestedOverlaysWorkflow.kt @@ -7,9 +7,11 @@ import com.squareup.workflow1.Snapshot import com.squareup.workflow1.StatefulWorkflow import com.squareup.workflow1.ui.Screen import com.squareup.workflow1.ui.WorkflowUiExperimentalApi +import com.squareup.workflow1.ui.container.BackButtonScreen import com.squareup.workflow1.ui.container.BodyAndOverlaysScreen import com.squareup.workflow1.ui.container.FullScreenModal +@OptIn(WorkflowUiExperimentalApi::class) object NestedOverlaysWorkflow : StatefulWorkflow() { data class State( val showTopBar: Boolean = true, @@ -46,15 +48,19 @@ object NestedOverlaysWorkflow : StatefulWorkflow() val outerSheet = if (!renderState.showOuterSheet) { null } else { + val closeOuter = context.eventHandler { state = state.copy(showOuterSheet = false) } FullScreenModal( - ButtonBar( - Button( - name = R.string.close, - onClick = context.eventHandler { state = state.copy(showOuterSheet = false) } + BackButtonScreen( + ButtonBar( + Button( + name = R.string.close, + onClick = closeOuter + ), + context.toggleInnerSheetButton(renderState), + color = android.R.color.holo_green_light, + showEditText = true, ), - context.toggleInnerSheetButton(renderState), - color = android.R.color.holo_green_light, - showEditText = true + onBackPressed = closeOuter ) ) } @@ -62,20 +68,24 @@ object NestedOverlaysWorkflow : StatefulWorkflow() val innerSheet = if (!renderState.showInnerSheet) { null } else { + val closeInner = context.eventHandler { state = state.copy(showInnerSheet = false) } FullScreenModal( - ButtonBar( - Button( - name = R.string.close, - onClick = context.eventHandler { state = state.copy(showInnerSheet = false) } + BackButtonScreen( + ButtonBar( + Button( + name = R.string.close, + onClick = closeInner + ), + toggleTopBarButton, + toggleBottomBarButton, + Button( + name = R.string.nuke, + onClick = context.eventHandler { state = State(nuked = true) } + ), + color = android.R.color.holo_red_light, + showEditText = true ), - toggleTopBarButton, - toggleBottomBarButton, - Button( - name = R.string.nuke, - onClick = context.eventHandler { state = State(nuked = true) } - ), - color = android.R.color.holo_red_light, - showEditText = true + onBackPressed = closeInner ) ) } diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index d403a64cb6..6f20158a91 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -366,7 +366,7 @@ public final class com/squareup/workflow1/ui/container/BackStackContainer$SavedS public final class com/squareup/workflow1/ui/container/LayeredDialogSessions { public static final field Companion Lcom/squareup/workflow1/ui/container/LayeredDialogSessions$Companion; public synthetic fun (Landroid/content/Context;Lkotlinx/coroutines/flow/StateFlow;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function0;Lkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun getAllowEvents ()Z + public final fun getAllowBodyEvents ()Z public final fun onAttachedToWindow (Ljava/lang/String;Landroid/view/View;)V public final fun onDetachedFromWindow ()V public final fun onRestoreInstanceState (Lcom/squareup/workflow1/ui/container/LayeredDialogSessions$SavedState;)V diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt index a9e4929394..5a472b65d4 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt @@ -84,11 +84,11 @@ internal class BodyAndOverlaysContainer @JvmOverloads constructor( } override fun dispatchTouchEvent(event: MotionEvent): Boolean { - return !dialogs.allowEvents || super.dispatchTouchEvent(event) + return !dialogs.allowBodyEvents || super.dispatchTouchEvent(event) } override fun dispatchKeyEvent(event: KeyEvent): Boolean { - return !dialogs.allowEvents || super.dispatchKeyEvent(event) + return !dialogs.allowBodyEvents || super.dispatchKeyEvent(event) } override fun onSaveInstanceState(): Parcelable { diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogCollator.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogCollator.kt index 3350b4dac0..0919db3e91 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogCollator.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogCollator.kt @@ -60,19 +60,28 @@ internal class DialogSessionUpdate( * and [DialogCollator.scheduleUpdates] * * @param existingSessions [DialogSession] instances that were running before the pending update + * + * @param onRootUpdateFinished honored only for the outermost [LayeredDialogSessions] caller. + * If provided, called with the complete set of [DialogSession] created by + * [DialogCollator.scheduleUpdates], not just those of the client identified by [id]. */ @WorkflowUiExperimentalApi internal fun ViewEnvironment.establishDialogCollator( id: UUID, - existingSessions: List + existingSessions: List, + onRootUpdateFinished: ((List) -> Unit)? ): ViewEnvironment { val collatorOrNull = map[DialogCollator] val collator = (collatorOrNull as? DialogCollator) ?: DialogCollator() - collator.expectedUpdates++ collator.establishedSessions.add( - IdAndSessions(id, existingSessions) + if (collator.expectedUpdates == 0) { + IdAndSessions(id, existingSessions, onRootUpdateFinished) + } else { + IdAndSessions(id, existingSessions) + } ) + collator.expectedUpdates++ return if (collatorOrNull == null) this + (DialogCollator to collator) else this } @@ -115,7 +124,8 @@ internal class DialogCollator { */ internal class IdAndSessions( val id: UUID, - val sessions: List + val sessions: List, + val onRootUpdateFinished: ((List) -> Unit)? = null ) /** @@ -201,6 +211,7 @@ internal class DialogCollator { // Z index of the dialog session being updated. var updatingSessionIndex = 0 + val allNewSessions = mutableListOf() allUpdates.forEach { idAndUpdates -> val updatedSessions = mutableListOf() @@ -240,8 +251,10 @@ internal class DialogCollator { updatingSessionIndex++ } idAndUpdates.onSessionsUpdated(updatedSessions) + allNewSessions += updatedSessions } + establishedSessions.first().onRootUpdateFinished?.invoke(allNewSessions) establishedSessions.clear() allUpdates.clear() } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogSession.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogSession.kt index d5fa1a0400..037f2ab500 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogSession.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/DialogSession.kt @@ -9,7 +9,7 @@ import android.view.KeyEvent.ACTION_UP import android.view.KeyEvent.KEYCODE_BACK import android.view.KeyEvent.KEYCODE_ESCAPE import android.view.MotionEvent -import android.view.Window +import android.view.Window.Callback import androidx.core.view.doOnAttach import androidx.core.view.doOnDetach import androidx.lifecycle.DefaultLifecycleObserver @@ -29,7 +29,8 @@ import com.squareup.workflow1.ui.androidx.WorkflowSavedStateRegistryAggregator internal class DialogSession( private val stateRegistryAggregator: WorkflowSavedStateRegistryAggregator, initialOverlay: Overlay, - holder: OverlayDialogHolder + holder: OverlayDialogHolder, + private val getParentLifecycleOwner: () -> LifecycleOwner ) { // Note similar code in LayeredDialogSessions private var allowEvents = true @@ -87,10 +88,11 @@ internal class DialogSession( private val KeyEvent.isBackPress: Boolean get() = (keyCode == KEYCODE_BACK || keyCode == KEYCODE_ESCAPE) && action == ACTION_UP - fun initAndShowDialog( - parentLifecycleOwner: LifecycleOwner, - initialEnvironment: ViewEnvironment - ) { + /** + * One time call to set up our brand new [OverlayDialogHolder] instance. + * This will be followed by one time calls to [showNewDialog] and [destroyDialog]. + */ + fun initNewDialog(initialEnvironment: ViewEnvironment) { // Prime the pump, make the first call to OverlayDialogHolder.show to update // the newly created Dialog to reflect the first rendering. Note that below // in this method we also have to apply initialOverlay to the Dialog itself @@ -102,7 +104,7 @@ internal class DialogSession( dialog.window?.let { window -> val realWindowCallback = window.callback - window.callback = object : Window.Callback by realWindowCallback { + window.callback = object : Callback by realWindowCallback { override fun dispatchTouchEvent(event: MotionEvent): Boolean { return !allowEvents || realWindowCallback.dispatchTouchEvent(event) } @@ -125,7 +127,18 @@ internal class DialogSession( } } } + } + + /** + * One time call to show the managed [Dialog][OverlayDialogHolder.dialog] for the first time. + * Called between [initNewDialog] and [destroyDialog]. + * + * See also [setVisible], used to dismiss and re-show an existing one. + */ + fun showNewDialog() { + val parentLifecycleOwner = getParentLifecycleOwner() + val dialog = holder.dialog dialog.show() // Fix for https://github.com/square/workflow-kotlin/issues/863, can't set this // until after show() is called. See kdoc in initialOverlay. @@ -154,7 +167,7 @@ internal class DialogSession( val lifecycle = parentLifecycleOwner.lifecycle val onDestroy = object : DefaultLifecycleObserver { override fun onDestroy(owner: LifecycleOwner) { - dismiss() + destroyDialog() } } @@ -179,16 +192,18 @@ internal class DialogSession( overlay: Overlay, environment: ViewEnvironment ) { - check(initialOverlay == null) { - "initAndShowDialog() must be called first. show() is for updates only." + if (initialOverlay != null) { + // Dialog hasn't been shown yet, keep the bootstrap hack fresh. + initialOverlay = overlay } + holder.show(overlay, environment) } /** * Used by [DialogCollator] to *temporarily* [dismiss][android.app.Dialog.dismiss] or * [show][android.app.Dialog.show] an existing [DialogSession] without triggering the - * other side effects of [dismiss], as a tool to update its z-index. + * other side effects of [destroyDialog], as a tool to update its z-index. */ fun setVisible(visible: Boolean) { if (visible) { @@ -202,7 +217,7 @@ internal class DialogSession( * We are never going to use this `Dialog` again. Tear down our lifecycle hooks * and dismiss it. */ - fun dismiss() { + fun destroyDialog() { with(holder.dialog) { // The dialog's views are about to be detached, and when that happens we want to transition // the dialog view's lifecycle to a terminal state even though the parent is probably still diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt index 5172fe6ad4..208ec760eb 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/LayeredDialogSessions.kt @@ -32,7 +32,7 @@ import java.util.UUID * by [BodyAndOverlaysContainer]. This class is public to allow implementation of * custom [Overlay]-based layouts if [BodyAndOverlaysContainer] is too restrictive. * - * - Provides an [allowEvents] field that reflects the presence or absence of Dialogs driven + * - Provides an [allowBodyEvents] field that reflects the presence or absence of Dialogs driven * by [ModalOverlay] * * - Makes [OverlayArea] available in the [ViewEnvironment], @@ -113,7 +113,36 @@ public class LayeredDialogSessions private constructor( private var sessions: List = emptyList() - public var allowEvents: Boolean = true + /** + * Reflects whether [onAttachedToWindow] has been called yet, so that + * we can avoid [showing][DialogSession.showNewDialog] `Dialog` windows + * before the `Activity` window is ready. + */ + private var attached = false + + /** + * Set by an outermost instance only. If we are in a root [update] call before + * [attached] is true, holds on to a list set of all [DialogSession]s, to be + * [shown][DialogSession.showNewDialog] at attach time instead of immediately. + * Necessary to ensure that view state is restored in the correct order -- + * the `Activity` `contentView` must be attached to its window before that of + * any `Dialog`. + * + * Note that this is a superset of [sessions], also includes sessions built by + * any nested [LayeredDialogSessions] instances during recursive [update] + * calls. + * + * https://github.com/square/workflow-kotlin/issues/1001 + */ + private var deferredShow: List? = null + + /** + * While this is true, clients should ignore UI events targeting the + * [body][BodyAndOverlaysScreen.body] view beneath the + * [Dialog][android.app.Dialog] windows they manage. It reflects + * that a [ModalOverlay] is (or soon will be) showing over the body. + */ + public var allowBodyEvents: Boolean = true private set(value) { val was = field field = value @@ -130,26 +159,33 @@ public class LayeredDialogSessions private constructor( * can use [ViewTreeLifecycleOwner][androidx.lifecycle.ViewTreeLifecycleOwner] as * usual. * - * @param updateBase function to be called before updating the dialogs, presumably - * to update the base view beneath the dialogs. The [ViewEnvironment] passed to the - * given function is enhanced with a [CoveredByModal] as appropriate, to ensure proper - * behavior of [allowEvents] of nested containers. + * @param updateBody function to be called before updating the dialogs, presumably + * to update the view for the [body][BodyAndOverlaysScreen.body] beneath the dialogs. + * The [ViewEnvironment] passed to the given function is enhanced with a [CoveredByModal] + * as appropriate, to ensure proper behavior of [allowBodyEvents] of nested containers. */ public fun update( overlays: List, viewEnvironment: ViewEnvironment, - updateBase: (environment: ViewEnvironment) -> Unit + updateBody: (environment: ViewEnvironment) -> Unit ) { // Set up a ViewEnvironment with the single DialogCollator instance that handles // this entire view hierarchy. See that class for details. - val envWithDialogManager = - viewEnvironment.establishDialogCollator(idInCollator, sessions) + val envWithDialogManager = viewEnvironment.establishDialogCollator( + id = idInCollator, + existingSessions = sessions, + onRootUpdateFinished = if (attached) { + null + } else { + { deferredShow = it } + } + ) val collator = envWithDialogManager[DialogCollator] val showingModals = overlays.any { it is ModalOverlay } - allowEvents = !showingModals + allowBodyEvents = !showingModals - updateBase( + updateBody( if (showingModals) envWithDialogManager + (CoveredByModal to true) else envWithDialogManager ) @@ -171,14 +207,19 @@ public class LayeredDialogSessions private constructor( holder.dialog.maintainBounds(holder.environment) { b -> updateBounds(b) } } - DialogSession(stateRegistryAggregator, overlay, holder).also { newSession -> - newSession.initAndShowDialog(getParentLifecycleOwner(), dialogEnv) - } + DialogSession(stateRegistryAggregator, overlay, holder, getParentLifecycleOwner) + .apply { + initNewDialog(dialogEnv) + + // Only show the new dialog immediately if we are already attached to the + // Activity window. + if (attached) showNewDialog() + } } } } collator.scheduleUpdates(idInCollator, updates) { updatedSessions -> - (sessions - updatedSessions.toSet()).forEach { it.dismiss() } + (sessions - updatedSessions.toSet()).forEach { it.destroyDialog() } // Drop the state registries for any keys that no longer exist since the last save. // Or really, drop everything except the remaining ones. stateRegistryAggregator.pruneAllChildRegistryOwnersExcept( @@ -201,10 +242,17 @@ public class LayeredDialogSessions private constructor( savedStateParentKey: String, view: View ) { + attached = true stateRegistryAggregator.attachToParentRegistry( savedStateParentKey, WorkflowAndroidXSupport.stateRegistryOwnerFromViewTreeOrContext(view) ) + + // If any dialogs were created before we were attached to the window, show them now. + deferredShow?.let { it -> + deferredShow = null + it.forEach { it.showNewDialog() } + } } /** @@ -213,6 +261,7 @@ public class LayeredDialogSessions private constructor( * Must be matched with a call to [onAttachedToWindow]. */ public fun onDetachedFromWindow() { + attached = false stateRegistryAggregator.detachFromParentRegistry() } @@ -266,7 +315,7 @@ public class LayeredDialogSessions private constructor( * * - The [view]'s [dispatchTouchEvent][View.dispatchTouchEvent] and * [dispatchKeyEvent][View.dispatchKeyEvent] methods should be overridden - * to honor [LayeredDialogSessions.allowEvents]. + * to honor [LayeredDialogSessions.allowBodyEvents]. * * - The [view]'s [onAttachedToWindow][View.onAttachedToWindow] and * [onDetachedFromWindow][View.onDetachedFromWindow] methods must call