From 01eb5764eafb3d6c51836a39a062f54826e51ef5 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Tue, 18 Jan 2022 09:25:09 -0800 Subject: [PATCH] WorkflowLayout.start needs a Lifecycle, BackStackScreen is supported. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a lot of Samsung devices running Anroid 12 / API 30, the sketchy `WorkflowLayout.takeWhileAttached` method at the heart of `WorkflowLayout.start` was having surprising effects -- we'd see redundant calls to `OnAttachStateChangeListener.onViewAttachedToWindow`. The problem goes away if we get more conventional and use the new `repeatOnLifecycle` function as described in [this blog post](https://medium.com/androiddevelopers/a-safer-way-to-collect-flows-from-android-uis-23080b1f8bda). The old methods are deprecated and will be deleted soon. Making this change required making our `BackPressHandler` mechanism more robust, as it too was pretending that `OnAttachStateChangeListener` was an adequate lifecycle. Specifically, we were relying on `onViewAttachedToWindow` to be called in a predicable order, which stopped being the case with the move away from `WorkflowLayout.takeWhileAttached`. This PR changes things so that… - We register handlers immediately, so that we can count on them being stacked in the expected order - We lean on `OnAttachStateChangeListener` solely to enable and disable the `BackPressHandler` associated with a view - And take advantage of our recently added support for `ViewTreeLifecycleOwner` for clean up `ModalContainer`'s contribution to coping with the back button also had to be beefed up, for similar reasons. It needs to ensure that back button events are blocked by modal windows, just like any other event, which is tricky with the singleton `OnBackPressedDispatcher` that androidx provides. It does this by putting a no-op handler in place as a backstop for any set by modal renderings. Its bespoke code for doing this was also inadvertently relying on the order of `onViewAttachedToWindow` calls. We now achieve this by wrapping modal renderings with a reliable `BackStackScreen`, which is promoted from the sample container set. --- .../HelloBindingActivity.kt | 5 +- .../HelloComposeWorkflowActivity.kt | 2 +- .../InlineRenderingActivity.kt | 4 +- .../NestedRenderingsActivity.kt | 5 +- .../sample/container/BackButtonScreen.kt | 23 --------- .../sample/container/BackButtonViewFactory.kt | 31 ------------ .../sample/container/SampleContainers.kt | 2 +- .../sample/poetryapp/PoetryActivity.kt | 2 +- .../squareup/sample/ravenapp/RavenActivity.kt | 2 +- .../hellobackbutton/AreYouSureWorkflow.kt | 2 +- .../HelloBackButtonActivity.kt | 2 +- .../sample/dungeon/DungeonActivity.kt | 2 +- .../HelloWorkflowFragment.kt | 4 +- .../helloworkflow/HelloWorkflowActivity.kt | 2 +- .../stubvisibility/StubVisibilityActivity.kt | 2 +- .../squareup/sample/TicTacToeEspressoTest.kt | 9 +++- .../sample/mainactivity/TicTacToeActivity.kt | 2 +- .../com/squareup/sample/todo/ToDoActivity.kt | 2 +- .../workflow1/ui/modal/ModalViewContainer.kt | 22 ++++++--- .../ui/backstack/BackStackScreenTest.kt | 2 +- workflow-ui/core-android/api/core-android.api | 12 +++++ .../squareup/workflow1/ui/BackButtonScreen.kt | 43 ++++++++++++++++ .../squareup/workflow1/ui/BackPressHandler.kt | 47 ++++++++++++------ .../squareup/workflow1/ui/WorkflowLayout.kt | 49 +++++++++++++++++-- 24 files changed, 171 insertions(+), 107 deletions(-) delete mode 100644 samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonScreen.kt delete mode 100644 samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt create mode 100644 workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackButtonScreen.kt diff --git a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposebinding/HelloBindingActivity.kt b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposebinding/HelloBindingActivity.kt index 4adc0cb2dd..73be69fdb2 100644 --- a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposebinding/HelloBindingActivity.kt +++ b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposebinding/HelloBindingActivity.kt @@ -36,10 +36,7 @@ class HelloBindingActivity : AppCompatActivity() { val model: HelloBindingModel by viewModels() setContentView( WorkflowLayout(this).apply { - start( - renderings = model.renderings, - environment = viewEnvironment - ) + start(lifecycle, model.renderings, viewEnvironment) } ) } diff --git a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/HelloComposeWorkflowActivity.kt b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/HelloComposeWorkflowActivity.kt index f666a1690f..b6ea8789db 100644 --- a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/HelloComposeWorkflowActivity.kt +++ b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/HelloComposeWorkflowActivity.kt @@ -17,7 +17,7 @@ class HelloComposeWorkflowActivity : AppCompatActivity() { super.onCreate(savedInstanceState) val model: HelloComposeModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings) } ) } diff --git a/samples/compose-samples/src/main/java/com/squareup/sample/compose/inlinerendering/InlineRenderingActivity.kt b/samples/compose-samples/src/main/java/com/squareup/sample/compose/inlinerendering/InlineRenderingActivity.kt index ae13885466..a2dd224e63 100644 --- a/samples/compose-samples/src/main/java/com/squareup/sample/compose/inlinerendering/InlineRenderingActivity.kt +++ b/samples/compose-samples/src/main/java/com/squareup/sample/compose/inlinerendering/InlineRenderingActivity.kt @@ -21,9 +21,7 @@ class InlineRenderingActivity : AppCompatActivity() { val model: HelloBindingModel by viewModels() setContentView( - WorkflowLayout(this).apply { - start(renderings = model.renderings) - } + WorkflowLayout(this).apply { start(lifecycle, model.renderings) } ) } diff --git a/samples/compose-samples/src/main/java/com/squareup/sample/compose/nestedrenderings/NestedRenderingsActivity.kt b/samples/compose-samples/src/main/java/com/squareup/sample/compose/nestedrenderings/NestedRenderingsActivity.kt index 0eecc3cbde..4886de82e1 100644 --- a/samples/compose-samples/src/main/java/com/squareup/sample/compose/nestedrenderings/NestedRenderingsActivity.kt +++ b/samples/compose-samples/src/main/java/com/squareup/sample/compose/nestedrenderings/NestedRenderingsActivity.kt @@ -38,10 +38,7 @@ class NestedRenderingsActivity : AppCompatActivity() { val model: NestedRenderingsModel by viewModels() setContentView( WorkflowLayout(this).apply { - start( - renderings = model.renderings, - environment = viewEnvironment - ) + start(lifecycle, model.renderings, viewEnvironment) } ) } diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonScreen.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonScreen.kt deleted file mode 100644 index d6314e309b..0000000000 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonScreen.kt +++ /dev/null @@ -1,23 +0,0 @@ -package com.squareup.sample.container - -import com.squareup.workflow1.ui.WorkflowUiExperimentalApi - -/** - * Adds optional back button handling to a [wrapped] rendering, possibly overriding that - * the wrapped rendering's own back button handler. - * - * @param override If `true`, [onBackPressed] is set as the - * [backPressedHandler][android.view.View.backPressedHandler] after - * the [wrapped] rendering's view is built / updated. If false, ours - * is set afterward, to allow the wrapped rendering to take precedence. - * Defaults to `false`. - * - * @param onBackPressed The function to fire when the device back button - * is pressed, or null to set no handler. Defaults to `null`. - */ -@WorkflowUiExperimentalApi -data class BackButtonScreen( - val wrapped: W, - val override: Boolean = false, - val onBackPressed: (() -> Unit)? = null -) diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt deleted file mode 100644 index 0c2e5569c4..0000000000 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/BackButtonViewFactory.kt +++ /dev/null @@ -1,31 +0,0 @@ -package com.squareup.sample.container - -import com.squareup.workflow1.ui.DecorativeViewFactory -import com.squareup.workflow1.ui.ViewFactory -import com.squareup.workflow1.ui.WorkflowUiExperimentalApi -import com.squareup.workflow1.ui.backPressedHandler - -/** - * [ViewFactory] that performs the work required by [BackButtonScreen]. - */ -@WorkflowUiExperimentalApi -object BackButtonViewFactory : ViewFactory> -by DecorativeViewFactory( - type = BackButtonScreen::class, - map = { outer -> outer.wrapped }, - doShowRendering = { view, innerShowRendering, outerRendering, viewEnvironment -> - if (!outerRendering.override) { - // Place our handler before invoking innerShowRendering, so that - // its later calls to view.backPressedHandler will take precedence - // over ours. - view.backPressedHandler = outerRendering.onBackPressed - } - - innerShowRendering.invoke(outerRendering.wrapped, viewEnvironment) - - if (outerRendering.override) { - // Place our handler after invoking innerShowRendering, so that ours wins. - view.backPressedHandler = outerRendering.onBackPressed - } - } -) diff --git a/samples/containers/android/src/main/java/com/squareup/sample/container/SampleContainers.kt b/samples/containers/android/src/main/java/com/squareup/sample/container/SampleContainers.kt index 6ce62e1cef..cadef2d832 100644 --- a/samples/containers/android/src/main/java/com/squareup/sample/container/SampleContainers.kt +++ b/samples/containers/android/src/main/java/com/squareup/sample/container/SampleContainers.kt @@ -8,5 +8,5 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi @OptIn(WorkflowUiExperimentalApi::class) val SampleContainers = ViewRegistry( - BackButtonViewFactory, OverviewDetailContainer, PanelContainer, ScrimContainer + OverviewDetailContainer, PanelContainer, ScrimContainer ) diff --git a/samples/containers/app-poetry/src/main/java/com/squareup/sample/poetryapp/PoetryActivity.kt b/samples/containers/app-poetry/src/main/java/com/squareup/sample/poetryapp/PoetryActivity.kt index bd29b51d64..a02a0c0728 100644 --- a/samples/containers/app-poetry/src/main/java/com/squareup/sample/poetryapp/PoetryActivity.kt +++ b/samples/containers/app-poetry/src/main/java/com/squareup/sample/poetryapp/PoetryActivity.kt @@ -26,7 +26,7 @@ class PoetryActivity : AppCompatActivity() { val model: PoetryModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings, viewRegistry) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings, viewRegistry) } ) } diff --git a/samples/containers/app-raven/src/main/java/com/squareup/sample/ravenapp/RavenActivity.kt b/samples/containers/app-raven/src/main/java/com/squareup/sample/ravenapp/RavenActivity.kt index 05dd90088f..6add9af69a 100644 --- a/samples/containers/app-raven/src/main/java/com/squareup/sample/ravenapp/RavenActivity.kt +++ b/samples/containers/app-raven/src/main/java/com/squareup/sample/ravenapp/RavenActivity.kt @@ -30,7 +30,7 @@ class RavenActivity : AppCompatActivity() { val model: RavenModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings, viewRegistry) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings, viewRegistry) } ) lifecycleScope.launch { diff --git a/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/AreYouSureWorkflow.kt b/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/AreYouSureWorkflow.kt index 732ccf0d1c..ad5691f402 100644 --- a/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/AreYouSureWorkflow.kt +++ b/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/AreYouSureWorkflow.kt @@ -1,7 +1,6 @@ package com.squareup.sample.hellobackbutton import android.os.Parcelable -import com.squareup.sample.container.BackButtonScreen import com.squareup.sample.hellobackbutton.AreYouSureWorkflow.Finished import com.squareup.sample.hellobackbutton.AreYouSureWorkflow.State import com.squareup.sample.hellobackbutton.AreYouSureWorkflow.State.Quitting @@ -10,6 +9,7 @@ import com.squareup.workflow1.Snapshot import com.squareup.workflow1.StatefulWorkflow import com.squareup.workflow1.WorkflowAction.Companion.noAction import com.squareup.workflow1.action +import com.squareup.workflow1.ui.BackButtonScreen import com.squareup.workflow1.ui.WorkflowUiExperimentalApi import com.squareup.workflow1.ui.modal.AlertContainerScreen import com.squareup.workflow1.ui.modal.AlertScreen diff --git a/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/HelloBackButtonActivity.kt b/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/HelloBackButtonActivity.kt index 084658a45a..56a54e639d 100644 --- a/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/HelloBackButtonActivity.kt +++ b/samples/containers/hello-back-button/src/main/java/com/squareup/sample/hellobackbutton/HelloBackButtonActivity.kt @@ -27,7 +27,7 @@ class HelloBackButtonActivity : AppCompatActivity() { val model: HelloBackButtonModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings, viewRegistry) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings, viewRegistry) } ) lifecycleScope.launch { diff --git a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonActivity.kt b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonActivity.kt index 4ce4b130b1..5cea9ac9a5 100644 --- a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonActivity.kt +++ b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonActivity.kt @@ -17,7 +17,7 @@ class DungeonActivity : AppCompatActivity() { val model: TimeMachineModel by viewModels { component.timeMachineModelFactory } setContentView( - WorkflowLayout(this).apply { start(model.renderings, component.viewRegistry) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings, component.viewRegistry) } ) } } diff --git a/samples/hello-workflow-fragment/src/main/java/com/squareup/sample/helloworkflowfragment/HelloWorkflowFragment.kt b/samples/hello-workflow-fragment/src/main/java/com/squareup/sample/helloworkflowfragment/HelloWorkflowFragment.kt index ccd6787034..de9723c629 100644 --- a/samples/hello-workflow-fragment/src/main/java/com/squareup/sample/helloworkflowfragment/HelloWorkflowFragment.kt +++ b/samples/hello-workflow-fragment/src/main/java/com/squareup/sample/helloworkflowfragment/HelloWorkflowFragment.kt @@ -23,10 +23,10 @@ class HelloWorkflowFragment : Fragment() { // This ViewModel will survive configuration changes. It's instantiated // by the first call to ViewModelProvider.get(), and that original instance is returned by // succeeding calls, until this Fragment session ends. - val model: HelloViewModel = ViewModelProvider(this).get(HelloViewModel::class.java) + val model: HelloViewModel = ViewModelProvider(this)[HelloViewModel::class.java] return WorkflowLayout(inflater.context).apply { - start(model.renderings) + start(lifecycle, model.renderings) } } } diff --git a/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt b/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt index 99318b4eb0..4c6447eb40 100644 --- a/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt +++ b/samples/hello-workflow/src/main/java/com/squareup/sample/helloworkflow/HelloWorkflowActivity.kt @@ -21,7 +21,7 @@ class HelloWorkflowActivity : AppCompatActivity() { // succeeding calls. val model: HelloViewModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings) } ) } } diff --git a/samples/stub-visibility/src/main/java/com/squareup/sample/stubvisibility/StubVisibilityActivity.kt b/samples/stub-visibility/src/main/java/com/squareup/sample/stubvisibility/StubVisibilityActivity.kt index 82d29114cc..cb8aa8f588 100644 --- a/samples/stub-visibility/src/main/java/com/squareup/sample/stubvisibility/StubVisibilityActivity.kt +++ b/samples/stub-visibility/src/main/java/com/squareup/sample/stubvisibility/StubVisibilityActivity.kt @@ -18,7 +18,7 @@ class StubVisibilityActivity : AppCompatActivity() { val model: StubVisibilityModel by viewModels() setContentView( - WorkflowLayout(this).apply { start(model.renderings) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings) } ) } } diff --git a/samples/tictactoe/app/src/androidTest/java/com/squareup/sample/TicTacToeEspressoTest.kt b/samples/tictactoe/app/src/androidTest/java/com/squareup/sample/TicTacToeEspressoTest.kt index 2517a7d212..4633acb156 100644 --- a/samples/tictactoe/app/src/androidTest/java/com/squareup/sample/TicTacToeEspressoTest.kt +++ b/samples/tictactoe/app/src/androidTest/java/com/squareup/sample/TicTacToeEspressoTest.kt @@ -36,6 +36,11 @@ import org.junit.rules.RuleChain import org.junit.runner.RunWith import java.util.concurrent.atomic.AtomicReference +/** + * This app is our most complex sample, which makes it a great candidate for + * integration testing — especially of modals, back stacks, back button handling, + * and view state management. + */ @OptIn(WorkflowUiExperimentalApi::class) @RunWith(AndroidJUnit4::class) class TicTacToeEspressoTest { @@ -155,7 +160,7 @@ class TicTacToeEspressoTest { .check(matches(isDisplayed())) } - @Test fun canGoBackInModalView() { + @Test fun canGoBackInModalViewAndSeeRestoredViewState() { // Log in and hit the 2fa screen. inAnyView(withId(R.id.login_email)).type("foo@2fa") inAnyView(withId(R.id.login_password)).type("password") @@ -168,7 +173,7 @@ class TicTacToeEspressoTest { inAnyView(withId(R.id.login_email)).check(matches(withText("foo@2fa"))) } - @Test fun configChangePreservesBackStackViewStateCache() { + @Test fun canGoBackInModalViewAfterConfigChangeAndSeeRestoredViewState() { // Log in and hit the 2fa screen. inAnyView(withId(R.id.login_email)).type("foo@2fa") inAnyView(withId(R.id.login_password)).type("password") diff --git a/samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/TicTacToeActivity.kt b/samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/TicTacToeActivity.kt index 38490140e2..ed1f6eb641 100644 --- a/samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/TicTacToeActivity.kt +++ b/samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/TicTacToeActivity.kt @@ -31,7 +31,7 @@ class TicTacToeActivity : AppCompatActivity() { idlingResource = component.idlingResource setContentView( - WorkflowLayout(this).apply { start(model.renderings, viewRegistry) } + WorkflowLayout(this).apply { start(lifecycle, model.renderings, viewRegistry) } ) lifecycleScope.launch { diff --git a/samples/todo-android/app/src/main/java/com/squareup/sample/todo/ToDoActivity.kt b/samples/todo-android/app/src/main/java/com/squareup/sample/todo/ToDoActivity.kt index ce33f281cf..8ab27deec2 100644 --- a/samples/todo-android/app/src/main/java/com/squareup/sample/todo/ToDoActivity.kt +++ b/samples/todo-android/app/src/main/java/com/squareup/sample/todo/ToDoActivity.kt @@ -26,7 +26,7 @@ class ToDoActivity : AppCompatActivity() { setContentView( WorkflowLayout(this).apply { - start(model.ensureWorkflow(traceFilesDir = filesDir), viewRegistry) + start(lifecycle, model.ensureWorkflow(traceFilesDir = filesDir), viewRegistry) } ) } diff --git a/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt b/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt index b63cac08e5..ea2cbd97f3 100644 --- a/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt +++ b/workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalViewContainer.kt @@ -10,11 +10,11 @@ import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.view.ViewGroup.LayoutParams.WRAP_CONTENT import androidx.annotation.IdRes +import com.squareup.workflow1.ui.BackButtonScreen import com.squareup.workflow1.ui.BuilderViewFactory import com.squareup.workflow1.ui.ViewEnvironment import com.squareup.workflow1.ui.ViewRegistry import com.squareup.workflow1.ui.WorkflowUiExperimentalApi -import com.squareup.workflow1.ui.backPressedHandler import com.squareup.workflow1.ui.bindShowRendering import com.squareup.workflow1.ui.buildView import com.squareup.workflow1.ui.modal.ModalViewContainer.Companion.binding @@ -62,22 +62,23 @@ public open class ModalViewContainer @JvmOverloads constructor( initialModalRendering: Any, initialViewEnvironment: ViewEnvironment ): DialogRef { + // Put a no-op backPressedHandler behind the given rendering, to + // ensure that the `onBackPressed` call below will not leak up to handlers + // that should be blocked by this modal session. + val wrappedRendering = BackButtonScreen(initialModalRendering) { } + val view = initialViewEnvironment[ViewRegistry] // Notice that we don't pass a custom initializeView function to set the // WorkflowLifecycleOwner here. ModalContainer will do that itself, on the parent of the view // created here. .buildView( - initialRendering = initialModalRendering, + initialRendering = wrappedRendering, initialViewEnvironment = initialViewEnvironment, contextForNewView = this.context, container = this ) .apply { start() - // If the modal's root view has no backPressedHandler, add a no-op one to - // ensure that the `onBackPressed` call below will not leak up to handlers - // that should be blocked by this modal session. - if (backPressedHandler == null) backPressedHandler = { } } return buildDialogForView(view) @@ -109,7 +110,14 @@ public open class ModalViewContainer @JvmOverloads constructor( } override fun updateDialog(dialogRef: DialogRef) { - with(dialogRef) { (extra as View).showRendering(modalRendering, viewEnvironment) } + with(dialogRef) { + // Have to preserve the wrapping done in buildDialog. (We can't put the + // BackButtonScreen in the DialogRef because the superclass needs to be + // able to do compatibility checks against it when deciding whether + // or not to update the existing dialog.) + val wrappedRendering = BackButtonScreen(modalRendering) { } + (extra as View).showRendering(wrappedRendering, viewEnvironment) + } } @PublishedApi diff --git a/workflow-ui/container-common/src/test/java/com/squareup/workflow1/ui/backstack/BackStackScreenTest.kt b/workflow-ui/container-common/src/test/java/com/squareup/workflow1/ui/backstack/BackStackScreenTest.kt index 8c1675fb03..3af34c9c14 100644 --- a/workflow-ui/container-common/src/test/java/com/squareup/workflow1/ui/backstack/BackStackScreenTest.kt +++ b/workflow-ui/container-common/src/test/java/com/squareup/workflow1/ui/backstack/BackStackScreenTest.kt @@ -6,7 +6,7 @@ import org.junit.Test import kotlin.test.assertFailsWith @OptIn(WorkflowUiExperimentalApi::class) -class BackStackScreenTest { +internal class BackStackScreenTest { @Test fun `top is last`() { assertThat(BackStackScreen(1, 2, 3, 4).top).isEqualTo(4) } diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 98d4d28112..9c8e1a1a09 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -11,6 +11,15 @@ public abstract interface class com/squareup/workflow1/ui/AndroidViewRendering { public abstract fun getViewFactory ()Lcom/squareup/workflow1/ui/ViewFactory; } +public final class com/squareup/workflow1/ui/BackButtonScreen : com/squareup/workflow1/ui/AndroidViewRendering { + public fun (Ljava/lang/Object;ZLkotlin/jvm/functions/Function0;)V + public synthetic fun (Ljava/lang/Object;ZLkotlin/jvm/functions/Function0;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun getOnBackPressed ()Lkotlin/jvm/functions/Function0; + public final fun getShadow ()Z + public fun getViewFactory ()Lcom/squareup/workflow1/ui/ViewFactory; + public final fun getWrapped ()Ljava/lang/Object; +} + public final class com/squareup/workflow1/ui/BackPressHandlerKt { public static final fun getBackPressedHandler (Landroid/view/View;)Lkotlin/jvm/functions/Function0; public static final fun onBackPressedDispatcherOwnerOrNull (Landroid/content/Context;)Landroidx/activity/OnBackPressedDispatcherOwner; @@ -203,8 +212,11 @@ public abstract interface class com/squareup/workflow1/ui/ViewStarter { public final class com/squareup/workflow1/ui/WorkflowLayout : android/widget/FrameLayout { public fun (Landroid/content/Context;Landroid/util/AttributeSet;)V public synthetic fun (Landroid/content/Context;Landroid/util/AttributeSet;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V + public final fun start (Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;)V public final fun start (Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewRegistry;)V + public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Landroidx/lifecycle/Lifecycle;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V public static synthetic fun start$default (Lcom/squareup/workflow1/ui/WorkflowLayout;Lkotlinx/coroutines/flow/Flow;Lcom/squareup/workflow1/ui/ViewEnvironment;ILjava/lang/Object;)V public final fun update (Ljava/lang/Object;Lcom/squareup/workflow1/ui/ViewEnvironment;)V } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackButtonScreen.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackButtonScreen.kt new file mode 100644 index 0000000000..eaeeb19433 --- /dev/null +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackButtonScreen.kt @@ -0,0 +1,43 @@ +package com.squareup.workflow1.ui + +/** + * Adds optional back button handling to a [wrapped] rendering, possibly overriding that + * the wrapped rendering's own back button handler. + * + * @param shadow If `true`, [onBackPressed] is set as the + * [backPressedHandler][android.view.View.backPressedHandler] after + * the [wrapped] rendering's view is built / updated, effectively overriding it. + * If false (the default), [onBackPressed] is set afterward, to allow the wrapped rendering to + * take precedence if it sets a `backPressedHandler` of its own -- the handler provided + * here serves as a default. + * + * @param onBackPressed The function to fire when the device back button + * is pressed, or null to set no handler -- or clear a handler that was set previously. + * Defaults to `null`. + */ +@WorkflowUiExperimentalApi +public class BackButtonScreen( + public val wrapped: W, + public val shadow: Boolean = false, + public val onBackPressed: (() -> Unit)? = null +) : AndroidViewRendering> { + override val viewFactory: ViewFactory> = DecorativeViewFactory( + type = BackButtonScreen::class, + map = { outer -> outer.wrapped }, + doShowRendering = { view, innerShowRendering, outerRendering, viewEnvironment -> + if (!outerRendering.shadow) { + // Place our handler before invoking innerShowRendering, so that + // its later calls to view.backPressedHandler will take precedence + // over ours. + view.backPressedHandler = outerRendering.onBackPressed + } + + innerShowRendering.invoke(outerRendering.wrapped, viewEnvironment) + + if (outerRendering.shadow) { + // Place our handler after invoking innerShowRendering, so that ours wins. + view.backPressedHandler = outerRendering.onBackPressed + } + } + ) +} diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackPressHandler.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackPressHandler.kt index 44a3780016..3f15233c37 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackPressHandler.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/BackPressHandler.kt @@ -6,6 +6,9 @@ import android.view.View import android.view.View.OnAttachStateChangeListener import androidx.activity.OnBackPressedCallback import androidx.activity.OnBackPressedDispatcherOwner +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.ViewTreeLifecycleOwner /** * A function passed to [View.backPressedHandler], to be called if the back @@ -18,8 +21,8 @@ public typealias BackPressHandler = () -> Unit * A function to be called if the device back button is pressed while this * view is attached to a window. * - * Implemented via a [OnBackPressedCallback], making this a - * last-registered-first-served mechanism. + * Implemented via [OnBackPressedDispatcher][androidx.activity.OnBackPressedDispatcher], + * making this a last-registered-first-served mechanism. */ @WorkflowUiExperimentalApi public var View.backPressedHandler: BackPressHandler? @@ -41,41 +44,53 @@ private val View.handlerWrapperOrNull * Uses the [androidx.activity.OnBackPressedDispatcher] to associate a [BackPressHandler] * with a [View]. * - * Registers [handler] whenever [view] is attached to a window, and removes it - * whenever [view] is detached. + * - Registers [handler] on [start] + * - Enables and disables it as [view] is attached to or detached from its window + * - De-registers it on [stop], or when its [lifecycle][ViewTreeLifecycleOwner] is destroyed */ @WorkflowUiExperimentalApi private class HandleBackPressWhenAttached( private val view: View, val handler: BackPressHandler -) : OnAttachStateChangeListener { - private val onBackPressedCallback = object : OnBackPressedCallback(true) { +) : OnAttachStateChangeListener, DefaultLifecycleObserver { + private val onBackPressedCallback = object : OnBackPressedCallback(false) { override fun handleOnBackPressed() { handler.invoke() } } fun start() { - view.addOnAttachStateChangeListener(this) - if (view.isAttachedToWindow) onViewAttachedToWindow(view) + view.context.onBackPressedDispatcherOwnerOrNull() + ?.let { owner -> + owner.onBackPressedDispatcher.addCallback(owner, onBackPressedCallback) + view.addOnAttachStateChangeListener(this) + if (view.isAttachedToWindow) onViewAttachedToWindow(view) + + // We enable the handler only while its view is attached to a window. + // This ensures that a temporarily removed view (e.g. for caching) + // does not participate in back button handling. + ViewTreeLifecycleOwner.get(view)?.lifecycle?.addObserver(this) + } } fun stop() { - if (view.isAttachedToWindow) onViewDetachedFromWindow(view) + onBackPressedCallback.remove() view.removeOnAttachStateChangeListener(this) + ViewTreeLifecycleOwner.get(view)?.lifecycle?.removeObserver(this) + } + + override fun onViewAttachedToWindow(attachedView: View) { + require(view === attachedView) + onBackPressedCallback.isEnabled = true } override fun onViewDetachedFromWindow(detachedView: View) { require(view === detachedView) - onBackPressedCallback.remove() + onBackPressedCallback.isEnabled = false } - override fun onViewAttachedToWindow(attachedView: View) { - require(view === attachedView) - view.context.onBackPressedDispatcherOwnerOrNull() - ?.let { owner -> - owner.onBackPressedDispatcher.addCallback(owner, onBackPressedCallback) - } + override fun onDestroy(owner: LifecycleOwner) { + stop() } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt index 5db79be210..9671c461f8 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt @@ -10,12 +10,17 @@ import android.view.View import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.widget.FrameLayout +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.coroutineScope +import androidx.lifecycle.repeatOnLifecycle import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch /** * A view that can be driven by a stream of renderings (and an optional [ViewRegistry]) @@ -47,7 +52,8 @@ public class WorkflowLayout( * Calls [WorkflowViewStub.update] on the [WorkflowViewStub] that is the only * child of this view. * - * This is the method called from [start]. It is exposed to allow clients to + * It's more common for a `Workflow`-based `Activity` or `Fragment` to use + * [start] than to call this method directly. It is exposed to allow clients to * make their own choices about how exactly to consume a stream of renderings. */ public fun update( @@ -63,23 +69,57 @@ public class WorkflowLayout( /** * This is the most common way to bootstrap a [Workflow][com.squareup.workflow1.Workflow] - * driven UI. Collects [renderings], and calls [start] with each one and [environment]. + * driven UI. Collects [renderings], and calls [update] with each one and [environment]. + * + * @param [lifecycle] the lifecycle that defines when and how this view should be updated. + * Typically this comes from `ComponentActivity.lifecycle` or `Fragment.lifecycle`. */ public fun start( + lifecycle: Lifecycle, renderings: Flow, environment: ViewEnvironment = ViewEnvironment() ) { - takeWhileAttached(renderings) { update(it, environment) } + // Just like https://medium.com/androiddevelopers/a-safer-way-to-collect-flows-from-android-uis-23080b1f8bda + lifecycle.coroutineScope.launch { + lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { + renderings.collect { update(it, environment) } + } + } } /** * A convenience overload that builds a [ViewEnvironment] around [registry], * for a bit less boilerplate. */ + public fun start( + lifecycle: Lifecycle, + renderings: Flow, + registry: ViewRegistry + ) { + start(lifecycle, renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) + } + + @Deprecated( + "Use a variant that takes a Lifecycle argument", + ReplaceWith("start(lifecycle, renderings, environment)") + ) + public fun start( + renderings: Flow, + environment: ViewEnvironment = ViewEnvironment() + ) { + @Suppress("DEPRECATION") + takeWhileAttached(renderings) { update(it, environment) } + } + + @Deprecated( + "Use a variant that takes a Lifecycle argument", + ReplaceWith("start(lifecycle, renderings, registry)") + ) public fun start( renderings: Flow, registry: ViewRegistry ) { + @Suppress("DEPRECATION") start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) } @@ -135,7 +175,10 @@ public class WorkflowLayout( /** * Subscribes [update] to [source] only while this [View] is attached to a window. + * Deprecated, leads to redundant calls to OnAttachStateChangeListener.onViewAttachedToWindow. + * To be deleted along with its callers. */ + @Deprecated("Do not use.") private fun View.takeWhileAttached( source: Flow, update: (S) -> Unit