Skip to content

Commit

Permalink
WorkflowLayout now relies on Lifecycle.
Browse files Browse the repository at this point in the history
Our 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.
  • Loading branch information
rjrjr committed Jan 18, 2022
1 parent 3d79f14 commit e0f10af
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 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 @@ -132,8 +132,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,10 +10,13 @@ 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

Expand Down Expand Up @@ -43,28 +46,51 @@ 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.
*/
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.
*/
public fun start(
lifecycle: Lifecycle,
renderings: Flow<Any>,
environment: ViewEnvironment = ViewEnvironment()
) {
takeWhileAttached(renderings) { show(it, environment) }
lifecycle.coroutineScope.launchWhenStarted {
renderings.collect { show(it, environment) }
}
}

private fun show(
Expand Down Expand Up @@ -131,7 +157,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 e0f10af

Please sign in to comment.