Enhance placeholder measurement with opt-in sizing policies #1110

Twinsen81 · web-flow · commit 6bc00486ed13 · 2025-09-24T14:47:46.000+02:00
diff --git a/media-placeholders/src/main/java/org/wordpress/aztec/placeholders/ComposePlaceholderAdapter.kt b/media-placeholders/src/main/java/org/wordpress/aztec/placeholders/ComposePlaceholderAdapter.kt
@@ -4,6 +4,90 @@ import androidx.compose.runtime.Composable
 import org.wordpress.aztec.AztecAttributes
 
 interface ComposePlaceholderAdapter : PlaceholderManager.PlaceholderAdapter {
+    /**
+     * Optional sizing hints for the manager.
+     *
+     * Usage:
+     * - Return [SizingPolicy.MatchWidthWrapContentHeight] if your content should
+     *   match the editor width and wrap to its intrinsic height. The manager will
+     *   pre-measure once offscreen to obtain the final height and avoid flicker.
+     * - Return [SizingPolicy.AspectRatio] for media with known aspect ratio. The
+     *   manager calculates height = width * ratio without composition.
+     * - Return [SizingPolicy.FixedHeightPx] for fixed-height embeds.
+     * - Return [SizingPolicy.Unknown] to keep legacy behavior; the manager will
+     *   call your existing [calculateHeight] implementation.
+     */
+    fun sizingPolicy(attrs: AztecAttributes): SizingPolicy = SizingPolicy.Unknown
+
+    /**
+     * Optional hook to compute a final height before first paint.
+     *
+     * When to use:
+     * - Your content height depends on Compose measurement (e.g., text wrapping)
+     *   and you want a single pass without interim sizes.
+     *
+     * How it works:
+     * - Manager provides a [measurer] that composes your content offscreen at an
+     *   exact width and returns its measured height in pixels.
+     * - Return that value to have the placeholder sized correctly up-front.
+     * - Return null to let the manager fall back to [sizingPolicy] or legacy
+     *   [calculateHeight].
+     *
+     * Notes:
+     * - Runs on the main thread. Do not perform long blocking work here.
+     * - Keep the content passed to [measurer.measure] minimal (only what affects
+     *   size) to make pre-measure cheap.
+     */
+    suspend fun preComposeMeasureHeight(
+        attrs: AztecAttributes,
+        widthPx: Int,
+        measurer: PlaceholderMeasurer
+    ): Int? = null
+
+    /**
+     * Optional spacing added after the placeholder, in pixels.
+     *
+     * This increases the reserved text-flow height while keeping the overlay
+     * view at the content height, producing a visual margin below the embed
+     * without an extra redraw.
+     */
+    fun bottomSpacingPx(attrs: AztecAttributes): Int = 0
+
+    /** Abstraction to measure Compose content offscreen at an exact width. */
+    interface PlaceholderMeasurer {
+        suspend fun measure(content: @Composable () -> Unit, widthPx: Int): Int
+    }
+
+    /** Sizing policy hints used by the manager to choose a measurement path. */
+    sealed interface SizingPolicy {
+        object Unknown : SizingPolicy
+        object MatchWidthWrapContentHeight : SizingPolicy
+        data class AspectRatio(val ratio: Float) : SizingPolicy
+        data class FixedHeightPx(val heightPx: Int) : SizingPolicy
+    }
+
+    /**
+     * Insets for positioning the overlay view within the reserved text area.
+     *
+     * This affects only the overlay position/size, not the reserved text-flow
+     * height. Use this to keep content away from edges (e.g., rounded corners)
+     * or to eliminate bottom inset if it causes clipping.
+     *
+     * Defaults match legacy behavior (10 px on each side). Return zeros for
+     * edge-to-edge rendering.
+     */
+    data class OverlayPadding(val left: Int, val top: Int, val right: Int, val bottom: Int)
+
+    fun overlayPaddingPx(attrs: AztecAttributes): OverlayPadding = OverlayPadding(10, 10, 10, 10)
+
+    /**
+     * Optional tiny positive adjustment added to the overlay height (pixels).
+     *
+     * Purpose: guard against 1 px rounding differences between pre-measure and
+     * runtime composition that could otherwise clip the last row/baseline.
+     * Leave at 0 unless you observe such edge cases.
+     */
+    fun contentHeightAdjustmentPx(attrs: AztecAttributes): Int = 0
     /**
      * Use this method to draw the placeholder using Jetpack Compose.
      * @param placeholderUuid the placeholder UUID
diff --git a/media-placeholders/src/main/java/org/wordpress/aztec/placeholders/ComposePlaceholderManager.kt b/media-placeholders/src/main/java/org/wordpress/aztec/placeholders/ComposePlaceholderManager.kt
@@ -4,10 +4,13 @@ package org.wordpress.aztec.placeholders
 
 import android.graphics.Rect
 import android.graphics.drawable.Drawable
+import android.os.Looper
 import android.text.Editable
 import android.text.Layout
 import android.text.Spanned
 import android.view.View
+import android.view.View.MeasureSpec
+import android.view.ViewGroup
 import android.view.ViewTreeObserver
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.height
@@ -17,6 +20,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.key
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.zIndex
 import androidx.core.content.ContextCompat
@@ -339,12 +343,23 @@ class ComposePlaceholderManager(
         val editorWidth = if (aztecText.width > 0) {
             aztecText.width - aztecText.paddingStart - aztecText.paddingEnd
         } else aztecText.maxImagesWidth
-        drawable.setBounds(
-            0,
-            0,
-            adapter.calculateWidth(attrs, editorWidth),
-            adapter.calculateHeight(attrs, editorWidth)
-        )
+
+        if (adapter.sizingPolicy(attrs) != ComposePlaceholderAdapter.SizingPolicy.Unknown) {
+            // New behavior with enhanced measuring
+            val widthPx = adapter.calculateWidth(attrs, editorWidth)
+            val heightPx = computeHeightPx(adapter, attrs, editorWidth, widthPx)
+            // Reserve additional flow space after the placeholder to visually separate following blocks
+            val flowHeight = heightPx + (adapter.bottomSpacingPx(attrs))
+            drawable.setBounds(0, 0, widthPx, flowHeight)
+        } else {
+            // Legacy behavior
+            drawable.setBounds(
+                0,
+                0,
+                adapter.calculateWidth(attrs, editorWidth),
+                adapter.calculateHeight(attrs, editorWidth)
+            )
+        }
         return drawable
     }
 
@@ -409,40 +424,160 @@ class ComposePlaceholderManager(
 
         val adapter = adapters[type]!!
         val windowWidth = parentTextViewRect.right - parentTextViewRect.left - EDITOR_INNER_PADDING
+
+        // Check if using new sizing policy or legacy behavior
+        val newComposeView = if (adapter.sizingPolicy(attrs) != ComposePlaceholderAdapter.SizingPolicy.Unknown) {
+            createComposeViewWithSizingPolicy(
+                adapter, attrs, uuid, windowWidth, parentTextViewRect, parentTextViewTopAndBottomOffset
+            )
+        } else {
+            createComposeViewWithLegacy(
+                adapter, attrs, uuid, windowWidth, parentTextViewRect, parentTextViewTopAndBottomOffset
+            )
+        }
+
+        // Check if view needs updating
+        val existingView = _composeViewState.value[uuid]
+        if (existingView != null &&
+            existingView.width == newComposeView.width &&
+            existingView.height == newComposeView.height &&
+            existingView.topMargin == newComposeView.topMargin &&
+            existingView.leftMargin == newComposeView.leftMargin &&
+            existingView.attrs == attrs
+        ) {
+            return
+        }
+
+        // Update compose view state
+        _composeViewState.value = _composeViewState.value.toMutableMap().apply {
+            this[uuid] = newComposeView
+        }
+    }
+
+    private suspend fun createComposeViewWithSizingPolicy(
+        adapter: ComposePlaceholderAdapter,
+        attrs: AztecAttributes,
+        uuid: String,
+        windowWidth: Int,
+        parentTextViewRect: Rect,
+        parentTextViewTopAndBottomOffset: Int
+    ): ComposeView {
+        val targetWidth = adapter.calculateWidth(attrs, windowWidth)
+        val measuredHeight = computeHeightPx(adapter, attrs, windowWidth, targetWidth)
+        val extraBottom = adapter.bottomSpacingPx(attrs)
+        val height = measuredHeight + extraBottom
+        parentTextViewRect.top += parentTextViewTopAndBottomOffset
+        parentTextViewRect.bottom = parentTextViewRect.top + height
+
+        val overlayPad = adapter.overlayPaddingPx(attrs)
+        val newLeftPadding = parentTextViewRect.left + overlayPad.left + aztecText.paddingStart
+        val newTopPadding = parentTextViewRect.top + overlayPad.top
+        val adjustedHeight = measuredHeight + adapter.contentHeightAdjustmentPx(attrs)
+
+        return ComposeView(
+            uuid = uuid,
+            width = targetWidth,
+            height = adjustedHeight,
+            topMargin = newTopPadding,
+            leftMargin = newLeftPadding,
+            visible = true,
+            adapterKey = adapter.type,
+            attrs = attrs
+        )
+    }
+
+    private suspend fun createComposeViewWithLegacy(
+        adapter: ComposePlaceholderAdapter,
+        attrs: AztecAttributes,
+        uuid: String,
+        windowWidth: Int,
+        parentTextViewRect: Rect,
+        parentTextViewTopAndBottomOffset: Int
+    ): ComposeView {
         val height = adapter.calculateHeight(attrs, windowWidth)
         parentTextViewRect.top += parentTextViewTopAndBottomOffset
         parentTextViewRect.bottom = parentTextViewRect.top + height
 
-        val box = _composeViewState.value[uuid]
         val newWidth = adapter.calculateWidth(attrs, windowWidth) - EDITOR_INNER_PADDING
         val newHeight = height - EDITOR_INNER_PADDING
         val padding = 10
         val newLeftPadding = parentTextViewRect.left + padding + aztecText.paddingStart
         val newTopPadding = parentTextViewRect.top + padding
-        box?.let { existingView ->
-            val widthSame = existingView.width == newWidth
-            val heightSame = existingView.height == newHeight
-            val topMarginSame = existingView.topMargin == newTopPadding
-            val leftMarginSame = existingView.leftMargin == newLeftPadding
-            val attrsSame = existingView.attrs == attrs
-            if (widthSame && heightSame && topMarginSame && leftMarginSame && attrsSame) {
-                return
+
+        return ComposeView(
+            uuid = uuid,
+            width = newWidth,
+            height = newHeight,
+            topMargin = newTopPadding,
+            leftMargin = newLeftPadding,
+            visible = true,
+            adapterKey = adapter.type,
+            attrs = attrs
+        )
+    }
+
+    private suspend fun computeHeightPx(
+        adapter: ComposePlaceholderAdapter,
+        attrs: AztecAttributes,
+        windowWidth: Int,
+        contentWidthPx: Int
+    ): Int =
+        when (val policy = adapter.sizingPolicy(attrs)) {
+            is ComposePlaceholderAdapter.SizingPolicy.FixedHeightPx -> policy.heightPx
+
+            is ComposePlaceholderAdapter.SizingPolicy.AspectRatio -> (policy.ratio * contentWidthPx).toInt()
+
+            ComposePlaceholderAdapter.SizingPolicy.MatchWidthWrapContentHeight ->
+                preMeasureHeight(adapter, attrs, contentWidthPx) ?: adapter.calculateHeight(attrs, windowWidth)
+
+            ComposePlaceholderAdapter.SizingPolicy.Unknown -> adapter.calculateHeight(attrs, windowWidth)
+        }
+
+    private suspend fun preMeasureHeight(
+        adapter: ComposePlaceholderAdapter,
+        attrs: AztecAttributes,
+        widthPx: Int
+    ): Int? {
+        // Pre-measure only on main thread. If not on main, fall back to legacy path
+        if (Looper.myLooper() != Looper.getMainLooper()) return null
+        val measurer = object : ComposePlaceholderAdapter.PlaceholderMeasurer {
+            override suspend fun measure(content: @Composable () -> Unit, widthPx: Int): Int {
+                if (!aztecText.isAttachedToWindow) return -1
+                val parent = aztecText.parent as? ViewGroup ?: return -1
+                val composeView = ComposeView(aztecText.context)
+                composeView.visibility = View.GONE
+                composeView.layoutParams = ViewGroup.LayoutParams(0, 0)
+                try {
+                    parent.addView(composeView)
+                    composeView.setContent {
+                        Box(
+                            Modifier
+                                .width(with(LocalDensity.current) { widthPx.toDp() })
+                        ) {
+                            content()
+                        }
+                    }
+                    val wSpec = MeasureSpec.makeMeasureSpec(widthPx, MeasureSpec.EXACTLY)
+                    val hSpec = MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED)
+                    composeView.measure(wSpec, hSpec)
+                    return composeView.measuredHeight
+                } catch (_: IllegalStateException) {
+                    return -1
+                } finally {
+                    parent.removeView(composeView)
+                }
             }
         }
-        _composeViewState.value = _composeViewState.value.let { state ->
-            val mutableState = state.toMutableMap()
-            mutableState[uuid] = ComposeView(
-                uuid = uuid,
-                width = newWidth,
-                height = newHeight,
-                topMargin = newTopPadding,
-                leftMargin = newLeftPadding,
-                visible = true,
-                adapterKey = adapter.type,
-                attrs = attrs
-            )
-            mutableState
+        // Let adapter compute/measure if it wants to
+        val fromAdapter = adapter.preComposeMeasureHeight(attrs, widthPx, measurer)
+        if (fromAdapter != null && fromAdapter >= 0) return fromAdapter
+        // If adapter did not implement it but hinted wrap content policy, measure the actual content once
+        if (adapter.sizingPolicy(attrs) == ComposePlaceholderAdapter.SizingPolicy.MatchWidthWrapContentHeight) {
+            val uuid = attrs.getValue(UUID_ATTRIBUTE)
+            val h = measurer.measure(content = { adapter.Placeholder(uuid, attrs) }, widthPx = widthPx)
+            return if (h >= 0) h else null
         }
+        return null
     }
 
     private fun validateAttributes(attributes: AztecAttributes): Boolean {
