Skip to content

Commit

Permalink
WorkflowLayout.start now requires a Lifecycle parameter.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rjrjr committed Jan 25, 2022
1 parent c84b71a commit 705d753
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,19 @@ 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)
}
)
}

class HelloBindingModel(savedState: SavedStateHandle) : ViewModel() {
@OptIn(WorkflowUiExperimentalApi::class)
val renderings: StateFlow<Any> by lazy {
renderWorkflowIn(
workflow = HelloWorkflow,
scope = viewModelScope,
savedStateHandle = savedState
workflow = HelloWorkflow,
scope = viewModelScope,
savedStateHandle = savedState
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ToDoActivity : AppCompatActivity() {

setContentView(
WorkflowLayout(this).apply {
start(model.ensureWorkflow(traceFilesDir = filesDir), viewRegistry)
start(lifecycle, model.ensureWorkflow(traceFilesDir = filesDir), viewRegistry)
}
)
}
Expand Down
3 changes: 3 additions & 0 deletions workflow-ui/core-android/api/core-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -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 <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V
public synthetic fun <init> (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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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(
Expand All @@ -61,26 +67,60 @@ public class WorkflowLayout(
}
}

@Deprecated(
"Use a variant that takes a Lifecycle argument",
ReplaceWith("start(lifecycle, renderings, environment)")
)
public fun start(
renderings: Flow<Any>,
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<Any>,
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<Any>,
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<Any>,
registry: ViewRegistry
) {
start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
start(lifecycle, renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
}

override fun onSaveInstanceState(): Parcelable {
Expand Down Expand Up @@ -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 <S : Any> View.takeWhileAttached(
source: Flow<S>,
update: (S) -> Unit
Expand Down

0 comments on commit 705d753

Please sign in to comment.