From b11e61e4199d7f145e89b8e9f76981ecc0dc54b3 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Mon, 13 Nov 2023 15:25:12 -0800 Subject: [PATCH] BREAKING: Stop stomping `View.layoutParams`, new parameter for `replaceViewInParent` `WorkflowViewStub.replaceViewInParent` now can be configured not to propagate `layoutParams`, by setting new property `propagatesLayoutParams` to `false`. When it's `true` (the default), the stub's `layoutParams` are passed as a new third argument to the (customizable) `replaceViewInParent` function, to make it clearer that this function bears responsibility for applying them. Motivated because `BodyAndOverlaysContainer` and `WorkflowLayout` were being dictatorial about the body view's layout params for no clear reason, leading to layout surprises. They're the first to opt out of `propagatesLayoutParams`. --- workflow-ui/core-android/api/core-android.api | 6 +++-- .../workflow1/ui/ScreenViewFactoryFinder.kt | 1 - .../squareup/workflow1/ui/WorkflowLayout.kt | 5 ++-- .../squareup/workflow1/ui/WorkflowViewStub.kt | 25 ++++++++++++++----- .../ui/container/BodyAndOverlaysContainer.kt | 6 ++--- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/workflow-ui/core-android/api/core-android.api b/workflow-ui/core-android/api/core-android.api index 007cdd5c2d..73d7db5201 100644 --- a/workflow-ui/core-android/api/core-android.api +++ b/workflow-ui/core-android/api/core-android.api @@ -270,13 +270,15 @@ public final class com/squareup/workflow1/ui/WorkflowViewStub : android/view/Vie public synthetic fun (Landroid/content/Context;Landroid/util/AttributeSet;IIILkotlin/jvm/internal/DefaultConstructorMarker;)V public final fun getActual ()Landroid/view/View; public final fun getInflatedId ()I - public final fun getReplaceOldViewInParent ()Lkotlin/jvm/functions/Function2; + public final fun getPropagatesLayoutParams ()Z + public final fun getReplaceOldViewInParent ()Lkotlin/jvm/functions/Function3; public final fun getUpdatesVisibility ()Z public fun getVisibility ()I public fun setBackground (Landroid/graphics/drawable/Drawable;)V public fun setId (I)V public final fun setInflatedId (I)V - public final fun setReplaceOldViewInParent (Lkotlin/jvm/functions/Function2;)V + public final fun setPropagatesLayoutParams (Z)V + public final fun setReplaceOldViewInParent (Lkotlin/jvm/functions/Function3;)V public final fun setUpdatesVisibility (Z)V public fun setVisibility (I)V public final fun show (Lcom/squareup/workflow1/ui/Screen;Lcom/squareup/workflow1/ui/ViewEnvironment;)V diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactoryFinder.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactoryFinder.kt index c55902c686..193428f91f 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactoryFinder.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/ScreenViewFactoryFinder.kt @@ -20,7 +20,6 @@ import com.squareup.workflow1.ui.container.EnvironmentScreen * by ScreenViewFactory( * buildView = { environment, context, _ -> * val view = MyBackStackContainer(context) - * .apply { layoutParams = (LayoutParams(MATCH_PARENT, MATCH_PARENT)) } * ScreenViewHolder(environment, view) { rendering, environment -> * view.update(rendering, environment) * } 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 7ed280da40..34f7d5426a 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 @@ -8,8 +8,6 @@ import android.os.Parcelable.Creator import android.util.AttributeSet import android.util.SparseArray 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.Lifecycle.State @@ -51,7 +49,8 @@ public class WorkflowLayout( private val showing: WorkflowViewStub = WorkflowViewStub(context).also { rootStub -> rootStub.updatesVisibility = false - addView(rootStub, ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) + rootStub.propagatesLayoutParams = false + addView(rootStub) } private var restoredChildState: SparseArray? = null diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt index 26e0b80604..0114871c5c 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/WorkflowViewStub.kt @@ -5,6 +5,7 @@ import android.graphics.drawable.Drawable import android.util.AttributeSet import android.view.View import android.view.ViewGroup +import android.view.ViewGroup.LayoutParams import androidx.annotation.IdRes import androidx.savedstate.SavedStateRegistryOwner import androidx.savedstate.findViewTreeSavedStateRegistryOwner @@ -85,6 +86,14 @@ public class WorkflowViewStub @JvmOverloads constructor( */ public var updatesVisibility: Boolean = true + /** + * If true, the [layoutParams][getLayoutParams] of this stub will be applied + * to new views as they are added to the stub's original parent. + * Specifically, the third parameter passed to [replaceOldViewInParent] + * will be non-null. + */ + public var propagatesLayoutParams: Boolean = true + /** * The id to be assigned to new views created by [update]. If the inflated id is * [View.NO_ID] (its default value), new views keep their original ids. @@ -121,15 +130,19 @@ public class WorkflowViewStub @JvmOverloads constructor( /** * Function called from [update] to replace this stub, or the current [actual], * with a new view. Can be updated to provide custom transition effects. + * The default implementation simply calls [ViewGroup.addView], including + * the `layoutParams` parameter if it is non-null. * - * Note that this method is responsible for copying the [layoutParams][getLayoutParams] - * from the stub to the new view. Also note that in a [WorkflowViewStub] that has never - * been updated, [actual] is the stub itself. + * @see propagatesLayoutParams */ - public var replaceOldViewInParent: (ViewGroup, View) -> Unit = { parent, newView -> + public var replaceOldViewInParent: ( + parent: ViewGroup, + newView: View, + layoutParams: LayoutParams? + ) -> Unit = { parent, newView, layoutParams -> val index = parent.indexOfChild(actual) parent.removeView(actual) - actual.layoutParams + layoutParams ?.let { parent.addView(newView, index, it) } ?: run { parent.addView(newView, index) } } @@ -241,7 +254,7 @@ public class WorkflowViewStub @JvmOverloads constructor( if (updatesVisibility) newView.visibility = visibility background?.let { newView.background = it } propagateSavedStateRegistryOwner(newView) - replaceOldViewInParent(parent, newView) + replaceOldViewInParent(parent, newView, layoutParams?.takeIf { propagatesLayoutParams }) } } diff --git a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt index ea5ef4c41c..b8588d09e7 100644 --- a/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt +++ b/workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/container/BodyAndOverlaysContainer.kt @@ -9,7 +9,6 @@ import android.os.Parcelable.Creator import android.util.AttributeSet import android.view.KeyEvent import android.view.MotionEvent -import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.widget.FrameLayout import com.squareup.workflow1.ui.Compatible @@ -42,8 +41,9 @@ internal class BodyAndOverlaysContainer @JvmOverloads constructor( */ private lateinit var savedStateParentKey: String - private val baseViewStub: WorkflowViewStub = WorkflowViewStub(context).also { - addView(it, ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) + private val baseViewStub: WorkflowViewStub = WorkflowViewStub(context).apply { + propagatesLayoutParams = false + addView(this) } private val dialogs = LayeredDialogSessions.forView(