Skip to content

fix: correct IterableEmbeddedView image height and scaleType (#997)#1040

Closed
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/997-embedded-view-image-height-zero
Closed

fix: correct IterableEmbeddedView image height and scaleType (#997)#1040
franco-zalamena-iterable wants to merge 1 commit intomasterfrom
fix/997-embedded-view-image-height-zero

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Apr 7, 2026

Summary

The embedded_message_image ImageView in the IterableEmbeddedView layout had a height of 0, causing images not to display when a Media URL was configured in a campaign.

Changes

  • Set android:layout_height="wrap_content" on the embedded message image view (was 0dp in card_view.xml)
  • Added android:adjustViewBounds="true" to maintain image aspect ratio
  • Added android:scaleType="fitCenter" for proper scaling that matches campaign preview
  • Added missing app:layout_constraintTop_toBottomOf="@id/embedded_message_image" on embedded_message_text_container in card_view.xml so it correctly positions below the image

Applied to all four layout files: layout/card_view.xml, layout-v21/card_view.xml, layout/banner_view.xml, and layout-v21/banner_view.xml.

Test plan

  • Verify embedded messages with Media URL display correctly
  • Verify image height matches campaign preview proportions
  • Verify embedded messages without media still display correctly
  • Test on various screen sizes and densities

Made with Cursor

Fix embedded_message_image ImageView having zero height when Media URL
is configured. Set layout_height to wrap_content, add adjustViewBounds=true
and scaleType=fitCenter to match campaign preview appearance.

Also add missing top constraint on embedded_message_text_container in
card_view.xml so it correctly positions below the image in ConstraintLayout.

Made-with: Cursor
@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

PR Analysis

Problem: The embedded_message_image ImageView in IterableEmbeddedView renders with 0 height, so campaign images configured with a Media URL never display. In card_view.xml, the root cause is a broken ConstraintLayout chain: the image has layout_height="0dp" (match constraints) with constraintBottom_toTopOf pointing at the text container, but the text container never points constraintTop_toBottomOf back at the image — so the image collapses to 0. In banner_view.xml the image had a fixed 80x80dp size, which should technically render but may also appear as 0 depending on how the image loading handles the view.

Ideal fix plan:

  • In card_view.xml: add the missing app:layout_constraintTop_toBottomOf="@id/embedded_message_image" on embedded_message_text_container to complete the constraint chain, which would let the 0dp height resolve correctly. Alternatively, switch to wrap_content with adjustViewBounds.
  • Add adjustViewBounds="true" and a sensible scaleType (e.g. fitCenter) to both card and banner layouts.
  • Consider adding maxHeight or app:layout_constraintHeight_max to prevent unbounded images from blowing out the layout.
  • Apply the same fixes to both layout/ and layout-v21/ variants.
  • Verify that banner_view (side-by-side layout) still looks correct with a dynamic image height instead of the fixed 80dp.

What the PR did:

  • Changed layout_height from 0dp to wrap_content in card_view and from 80dp to wrap_content in banner_view.
  • Added adjustViewBounds="true" and scaleType="fitCenter" to all four layout files.
  • Added the missing constraintTop_toBottomOf on the text container in card_view.
  • Applied changes consistently to both layout/ and layout-v21/ variants.

Assessment:

  • Root cause identified: partially — the PR correctly identified that the image height was 0 and fixed it, but the PR description frames the issue as the height simply being 0dp, when the actual root cause in card_view was a broken constraint chain (the text container was missing its top constraint back to the image). Adding the missing constraint alone would have been sufficient to fix the card_view without changing to wrap_content.
  • Fix correctness: partially correct — the fix will make images visible, which solves the immediate bug. However, switching to wrap_content without any maxHeight or constraintHeight_max means an arbitrarily tall source image could blow out the layout. The original 0dp + completed chain approach would have been more robust as it lets ConstraintLayout distribute space. Additionally, in the banner layout (side-by-side design), removing the fixed 80dp thumbnail size in favor of unbounded wrap_content could produce unexpected results with various image aspect ratios — the image may push the text off-screen or cause layout shifts.
  • Missed:
    • No maxHeight or app:layout_constraintHeight_max to guard against oversized images.
    • The banner_view change from fixed 80dp to unbounded wrap_content is risky for a side-by-side layout. A fixed or capped thumbnail width/height is more appropriate there.
    • Removing the constraintBottom_toTopOf from the image in card_view (it's still present) while also adding constraintTop_toBottomOf on the text container creates a bidirectional link that may cause unexpected sizing when combined with wrap_content instead of 0dp.
  • Wrong assessment: none — the direction is correct, just incomplete.
  • Tests: needed but missing — at minimum, screenshot or layout-level tests for embedded views with and without media URLs would help prevent regressions.

@franco-zalamena-iterable franco-zalamena-iterable deleted the fix/997-embedded-view-image-height-zero branch April 8, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant