From 705d753f260ecf705ab4996590c2a9e98a13f58c Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Tue, 18 Jan 2022 09:25:09 -0800 Subject: [PATCH] WorkflowLayout.start now requires a Lifecycle parameter. 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 `View.onAttachedToWindow`. 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. --- .../HelloBindingActivity.kt | 15 +++--- .../HelloComposeWorkflowActivity.kt | 2 +- .../InlineRenderingActivity.kt | 4 +- .../NestedRenderingsActivity.kt | 5 +- .../sample/poetryapp/PoetryActivity.kt | 2 +- .../squareup/sample/ravenapp/RavenActivity.kt | 2 +- .../HelloBackButtonActivity.kt | 2 +- .../sample/dungeon/DungeonActivity.kt | 2 +- .../HelloWorkflowFragment.kt | 4 +- .../helloworkflow/HelloWorkflowActivity.kt | 2 +- .../stubvisibility/StubVisibilityActivity.kt | 2 +- .../sample/mainactivity/TicTacToeActivity.kt | 2 +- .../com/squareup/sample/todo/ToDoActivity.kt | 2 +- workflow-ui/core-android/api/core-android.api | 3 ++ .../squareup/workflow1/ui/WorkflowLayout.kt | 49 +++++++++++++++++-- 15 files changed, 67 insertions(+), 31 deletions(-) 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 12821d38f7..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 @@ -35,12 +35,9 @@ class HelloBindingActivity : AppCompatActivity() { val model: HelloBindingModel by viewModels() setContentView( - WorkflowLayout(this).apply { - start( - renderings = model.renderings, - environment = viewEnvironment - ) - } + WorkflowLayout(this).apply { + start(lifecycle, model.renderings, viewEnvironment) + } ) } @@ -48,9 +45,9 @@ class HelloBindingActivity : AppCompatActivity() { @OptIn(WorkflowUiExperimentalApi::class) val renderings: StateFlow by lazy { renderWorkflowIn( - workflow = HelloWorkflow, - scope = viewModelScope, - savedStateHandle = savedState + workflow = HelloWorkflow, + scope = viewModelScope, + savedStateHandle = savedState ) } } 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/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/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/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/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 98d4d28112..7e90058650 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -203,8 +203,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/WorkflowLayout.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowLayout.kt index 6f66b2ffa8..778335939f 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( @@ -61,15 +67,48 @@ public class WorkflowLayout( } } + @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))) + } + /** * 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) } + } + } } /** @@ -77,10 +116,11 @@ public class WorkflowLayout( * for a bit less boilerplate. */ public fun start( + lifecycle: Lifecycle, renderings: Flow, registry: ViewRegistry ) { - start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) + start(lifecycle, renderings, ViewEnvironment(mapOf(ViewRegistry to registry))) } override fun onSaveInstanceState(): Parcelable { @@ -136,6 +176,7 @@ public class WorkflowLayout( /** * Subscribes [update] to [source] only while this [View] is attached to a window. */ + @Deprecated("this was a mistake") private fun View.takeWhileAttached( source: Flow, update: (S) -> Unit