Skip to content

Commit

Permalink
WorkflowLayout.start now relies on Lifecycle.
Browse files Browse the repository at this point in the history
Our sketchy homegrown `View.takeWhileAttached` somehow causes a strange issue
on API 30 devices where `View.onAttached` can be called twice, at least since
the recent change to introduce `View.start` landed in #602. We've seen
crashes on a variety of Samsung devices out in the wild, and can reproduce
the issue on API 30 AVDs via Android Studio's _Apply Changes and Restart
Activity_.

The fix is to be mainstream and use `Lifecycle`, which is added as a new
required parameter to `WorkflowLayout.start`. The old overloads are now
deprecated, and will be deleted soon.

We also promote `WorkflowLayout.show` to be publicly visible, to give apps the
option of taking control over how they collect their renderings.
  • Loading branch information
rjrjr committed Jan 19, 2022
1 parent 0347338 commit 2d789d1
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
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).also { contentView -> contentView.start(lifecycle, model.renderings) }
)
}
}
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
}

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 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
import androidx.lifecycle.repeatOnLifecycle

/**
* A view that can be driven by a stream of renderings (and an optional [ViewRegistry])
Expand Down Expand Up @@ -43,31 +48,68 @@ public class WorkflowLayout(

private var restoredChildState: SparseArray<Parcelable>? = null

@Suppress("DeprecatedCallableAddReplaceWith")
@Deprecated("Provide an androidx Lifecyle")
public fun start(
renderings: Flow<Any>,
registry: ViewRegistry
) {
@Suppress("DEPRECATION")
start(renderings, ViewEnvironment(mapOf(ViewRegistry to registry)))
}

/**
* Subscribes to [renderings], and uses [registry] to
* [build a new view][ViewRegistry.buildView] each time a new type of rendering is received,
* making that view the only child of this one.
*
* @see [show]
*/
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)))
}

@Deprecated("Provide an androidx Lifecyle")
public fun start(
renderings: Flow<Any>,
environment: ViewEnvironment = ViewEnvironment()
) {
takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis(renderings) { show(it, environment) }
}

/**
* Subscribes to [renderings], and uses the [ViewRegistry] in the given [environment] to
* [build a new view][ViewRegistry.buildView] each time a new type of rendering is received,
* making that view the only child of this one.
*
* @see [show]
*/
public fun start(
lifecycle: Lifecycle,
renderings: Flow<Any>,
environment: ViewEnvironment = ViewEnvironment()
) {
takeWhileAttached(renderings) { show(it, environment) }
lifecycle.coroutineScope.launch {
lifecycle.repeatOnLifecycle(Lifecycle.State.RESUMED) {
renderings.collect { show(it, environment) }
}
}
}

private fun show(
/**
* Called from [start] as each rendering is collected from its `renderings: Flow<Any>`
* parameter. Updates the current child view from [newRendering] and [environment]
* if [newRendering] is [compatible] with the current one, or else replaces the
* current view with a new one.
*
* This method is exposed as an alternative to [start], to offer clients complete control
* over collecting their renderings.
*/
public fun show(
newRendering: Any,
environment: ViewEnvironment
) {
Expand Down Expand Up @@ -131,7 +173,7 @@ public class WorkflowLayout(
/**
* Subscribes [update] to [source] only while this [View] is attached to a window.
*/
private fun <S : Any> View.takeWhileAttached(
private fun <S : Any> View.takeWhileAttachedIfYouAreAnIdiotNoDoNotUseThis(
source: Flow<S>,
update: (S) -> Unit
) {
Expand Down

0 comments on commit 2d789d1

Please sign in to comment.